public inbox for gentoo-catalyst@lists.gentoo.org
 help / color / mirror / Atom feed
From: antarus@gentoo.org
To: gentoo-catalyst@lists.gentoo.org
Subject: [gentoo-catalyst] [PATCH] Simplify a bit of code.
Date: Wed, 18 Dec 2013 22:07:45 -0800	[thread overview]
Message-ID: <1387433265-30905-1-git-send-email-antarus@gentoo.org> (raw)

From: Alec Warner <antarus@gentoo.org>

We want a default for the config file. We can handle this in main() by simply
setting myconfig to the default. If the user over-rides it with -c, we will take
the user provided value.

Delete a bunch of duplicate code.
Add docstring for parse_config.
Add warning that parse_config modifies global state.
Add TODO's about bad habits, like randomly calling sys.exit() in your code.

Calling stuff 'myFOO' is really annoying, particularly when you have a function such
as this and you have variables like:

myconfig
myconf
conf_values

and you have to tell them apart ;)

-A
---
 catalyst | 50 +++++++++++++++++++++-----------------------------
 1 file changed, 21 insertions(+), 29 deletions(-)

diff --git a/catalyst b/catalyst
index edc0b24..47efb90 100755
--- a/catalyst
+++ b/catalyst
@@ -57,11 +57,18 @@ def version():
 	print "Copyright 2008-2012 various authors"
 	print "Distributed under the GNU General Public License version 2.1\n"
 
-def parse_config(myconfig):
-	# search a couple of different areas for the main config file
-	myconf={}
-	config_file=""
-
+def parse_config(config_path):
+	"""Parse the catalyst config file and generate a targetmap.
+
+	TODO(antarus): DANGER: This file modifies global state (conf_values).
+
+	Args:
+		config_path - A path to a catalyst config file.
+	Returns:
+		None
+	"""
+	
+	# TODO(antarus): Move this outside of this function.
 	confdefaults = {
 		"distdir": "/usr/portage/distfiles",
 		"hash_function": "crc32",
@@ -75,33 +82,18 @@ def parse_config(myconfig):
 		"storedir": "/var/tmp/catalyst",
 		}
 
-	# first, try the one passed (presumably from the cmdline)
-	if myconfig:
-		if os.path.exists(myconfig):
-			print "Using command line specified Catalyst configuration file, "+myconfig
-			config_file=myconfig
-
-		else:
-			print "!!! catalyst: Could not use specified configuration file "+\
-				myconfig
-			sys.exit(1)
-
-	# next, try the default location
-	elif os.path.exists("/etc/catalyst/catalyst.conf"):
-		print "Using default Catalyst configuration file, /etc/catalyst/catalyst.conf"
-		config_file="/etc/catalyst/catalyst.conf"
 
-	# can't find a config file (we are screwed), so bail out
-	else:
-		print "!!! catalyst: Could not find a suitable configuration file"
+	try:
+		print "Trying config file '%s'" % config_path
+		cfg = modules.catalyst.config.ConfigParser(config_path)
+ 	except EnvironmentError as e:
+		print "Could not read config file '%s', got error '%s'" % (config_path, e)
+		# TODO(antarus): it should be discouraged to call sys.exit() from functions.
+		# One should call sys.exit() from main()..this isn't main().
 		sys.exit(1)
 
-	# now, try and parse the config file "config_file"
 	try:
-#		execfile(config_file, myconf, myconf)
-		myconfig = modules.catalyst.config.ConfigParser(config_file)
-		myconf.update(myconfig.get_values())
-
+		myconf.update(cfg.get_values())
 	except:
 		print "!!! catalyst: Unable to parse configuration file, "+myconfig
 		sys.exit(1)
@@ -258,7 +250,7 @@ if __name__ == "__main__":
 	debug=False
 	verbose=False
 	fetch=False
-	myconfig=""
+	myconfig="/etc/catalyst/catalyst.conf"
 	myspecfile=""
 	mycmdline=[]
 	myopts=[]
-- 
1.8.1.2



                 reply	other threads:[~2013-12-19  6:07 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1387433265-30905-1-git-send-email-antarus@gentoo.org \
    --to=antarus@gentoo.org \
    --cc=gentoo-catalyst@lists.gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox