public inbox for gentoo-catalyst@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-catalyst] [PATCH] Simplify a bit of code.
@ 2013-12-19  6:07 antarus
  0 siblings, 0 replies; only message in thread
From: antarus @ 2013-12-19  6:07 UTC (permalink / raw
  To: gentoo-catalyst

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



^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2013-12-19  6:07 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-19  6:07 [gentoo-catalyst] [PATCH] Simplify a bit of code antarus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox