This looks pretty good in general. But a few comments/questions. Most
of which are nitpicky little things, but there's a little bit of meat in
the middle :)
On Mon, 2008-05-19 at 11:58 -0400, David Huff wrote:
diff --git a/imgcreate/creator.py b/imgcreate/creator.py
index 1028e32..febb847 100644
--- a/imgcreate/creator.py
+++ b/imgcreate/creator.py
@@ -522,7 +523,7 @@ class ImageCreator(object):
(pkg, e))
for pkg in skipped_pkgs:
- print >> sys.stderr, "Skipping missing package '%s'"
% (pkg,)
+ logging.info("Skipping missing package '%s'" % (pkg,))
Maybe should be logging.warn?
@@ -537,7 +538,7 @@ class ImageCreator(object):
skipped_groups.append(group)
for group in skipped_groups:
- print >> sys.stderr, "Skipping missing group '%s'" %
(group.name,)
+ logging.info("Skipping missing group '%s'" %
(group.name,))
And the same.
+def handle_logfile(option, opt, val, parser, logger, stream):
+ try:
+ logfile = logging.handlers.RotatingFileHandler(val, "a",
+ LOGFILE_MAX_BYTES,
+ LOGFILE_N_BACKUPS)
A rotating log file seems like overkill here. Let's keep it just a
simple file.
+ logfile.setFormatter(logging.Formatter(FILE_FORMAT,
DATE_FORMAT))
Is there really a convincing reason not to just stick with the default
format handling?
+def setup_logging(parser = None):
+ """Set up the root logger and add logging options.
What happens if the utility doesn't call setup_logging()? It's
important that the behavior be basically the same for anyone who has
built something on top of the existing API who doesn't immediately go
and add the setup_logging() call
diff --git a/tools/livecd-creator b/tools/livecd-creator
index 7c08323..dddfddd 100755
--- a/tools/livecd-creator
+++ b/tools/livecd-creator
@@ -22,8 +22,10 @@ import os.path
import sys
import time
import optparse
+import logging
import imgcreate
+import imgcreate.debug
This import isn't needed anymore.
Jeremy