Merge lp:~marcoceppi/charm-tools/style-fixes into lp:~charmers/charm-tools/trunk

[Marco Ceppi]

This just fixes all the files to have the same indentation and "style" of coding. Making it easier to read and switch between files.

[Clint Byrum (clint-fewbar)]

Hey Marco, thanks for caring! Seriously, this needs to be done.

-1 because if you're going to do an indent level, do 4 spaces, not 2. I also would suggest doing a full PEP8 style push for all the python files, since that is the de-facto standard for python formatting. Should then include that in 'make check'. If you're going to do a style change, do it *all* in one commit, as these thing are really disruptive to things like trying to do a 'bzr annotate' on a file. Then don't let it regress so you never have to do another style change again.

review: Disapprove

[Marco Ceppi (marcoceppi)]

Thanks for the feedback clint. I'll add that feedback squash and re-open the merge request when I get done formatting all the files!

[lp:~marcoceppi/charm-tools/style-fixes]

170. By Marco Ceppi on 2013-02-25

Make all Python scripts follow PEP8, lint code during make check

[Marco Ceppi (marcoceppi)]

Updated to use PEP8 formatting, implemented Clint's feedback

[Mark Mims (mark-mims)]

lgtm

review: Approve

[Kapil Thangavelu (hazmat)]

This merge proposal broke the unit tests, and is *NOT* in the ppa because
the packaging there runs the unit tests.

On Thu, Mar 14, 2013 at 12:19 PM, Mark Mims <email address hidden> wrote:

> The proposal to merge lp:~marcoceppi/charm-tools/style-fixes into
> lp:charm-tools has been updated.
>
> Status: Approved => Merged
>
> For more details, see:
>
> https://code.launchpad.net/~marcoceppi/charm-tools/style-fixes/+merge/150374
> --
>
> https://code.launchpad.net/~marcoceppi/charm-tools/style-fixes/+merge/150374
> Your team charmers is subscribed to branch lp:charm-tools.
>

[Marco Ceppi (marcoceppi)]

Kapil, sorry for that it looks like there was something committed in upstream that failed the pep8 test. I've fixed that code so `make check` works properly now, see https://code.launchpad.net/~marcoceppi/charm-tools/pep8-fix/+merge/153585

=== modified file 'Makefile'
--- Makefile	2012-09-05 22:19:13 +0000
+++ Makefile	2013-02-25 21:57:30 +0000
@@ -35,3 +35,5 @@
 	tests/proof/test.sh
 	tests/create/test.sh
 	PYTHONPATH=helpers/python python helpers/python/charmhelpers/tests/test_charmhelpers.py
+	@echo PEP8 Lint of Python files
+	pep8 ./ `grep -rl '^#!/.*python' .`

=== modified file 'charm'
--- charm	2012-08-28 18:59:42 +0000
+++ charm	2013-02-25 21:57:30 +0000
@@ -9,10 +9,10 @@
 
 usage()
 {
-  echo "usage: $1 [ --help|-h ] [ script_name ]" >&2
-  echo script choices are: >&2
-  echo $(list_commands)|sed -e 's/^/    /' >&2
-  exit $2
+    echo "usage: $1 [ --help|-h ] [ script_name ]" >&2
+    echo script choices are: >&2
+    echo $(list_commands)|sed -e 's/^/    /' >&2
+    exit $2
 }
 
 list_commands()
@@ -40,9 +40,11 @@
 if [ -z "$script" ] ; then
     usage $(basename $0) 0
 fi
+
 if [ -x $script_home/$script ] ; then
-  shift
-  exec $script_home/$script $*
+    shift
+    exec $script_home/$script $*
 fi
+
 echo "abort: Unknown script $script" >&2
 usage $(basename $0) 2

=== modified file 'ez_setup.py'
--- ez_setup.py	2012-03-01 23:00:50 +0000
+++ ez_setup.py	2013-02-25 21:57:30 +0000
@@ -19,7 +19,8 @@
 """
 import sys
 DEFAULT_VERSION = "0.6c11"
-DEFAULT_URL     = "http://pypi.python.org/packages/%s/s/setuptools/" % sys.version[:3]
+DEFAULT_URL = "http://pypi.python.org/packages/%s/s/setuptools/" \
+                   % sys.version[:3]
 
 md5_data = {
     'setuptools-0.6b1-py2.3.egg': '8822caf901250d848b996b7f25c6e6ca',
@@ -66,9 +67,14 @@
     'setuptools-0.6c9-py2.6.egg': 'ca37b1ff16fa2ede6e19383e7b59245a',
 }
 
-import sys, os
-try: from hashlib import md5
-except ImportError: from md5 import md5
+import os
+import sys
+
+try:
+    from hashlib import md5
+except ImportError:
+    from md5 import md5
+
 
 def _validate_md5(egg_name, data):
     if egg_name in md5_data:
@@ -81,6 +87,7 @@
             sys.exit(2)
     return data
 
+
 def use_setuptools(
     version=DEFAULT_VERSION, download_base=DEFAULT_URL, to_dir=os.curdir,
     download_delay=15
@@ -96,17 +103,22 @@
     this routine will print a message to ``sys.stderr`` and raise SystemExit in
     an attempt to abort the calling script.
     """
-    was_imported = 'pkg_resources' in sys.modules or 'setuptools' in sys.modules
+    was_imported = 'pkg_resources' in sys.modules \
+                    or 'setuptools' in sys.modules
+
     def do_download():
-        egg = download_setuptools(version, download_base, to_dir, download_delay)
+        egg = download_setuptools(version, download_base, to_dir,
+            download_delay)
         sys.path.insert(0, egg)
-        import setuptools; setuptools.bootstrap_install_from = egg
+        import setuptools
+        setuptools.bootstrap_install_from = egg
     try:
         import pkg_resources
     except ImportError:
         return do_download()
     try:
-        pkg_resources.require("setuptools>="+version); return
+        pkg_resources.require("setuptools>=" + version)
+        return
     except pkg_resources.VersionConflict, e:
         if was_imported:
             print >>sys.stderr, (
@@ -122,19 +134,22 @@
     except pkg_resources.DistributionNotFound:
         return do_download()
 
+
 def download_setuptools(
     version=DEFAULT_VERSION, download_base=DEFAULT_URL, to_dir=os.curdir,
-    delay = 15
+    delay=15
 ):
     """Download setuptools from a specified location and return its filename
 
     `version` should be a valid setuptools version number that is available
     as an egg for download under the `download_base` URL (which should end
     with a '/'). `to_dir` is the directory where the egg will be downloaded.
-    `delay` is the number of seconds to pause before an actual download attempt.
+    `delay` is the number of seconds to pause before an actual download
+    attempt.
     """
-    import urllib2, shutil
-    egg_name = "setuptools-%s-py%s.egg" % (version,sys.version[:3])
+    import urllib2
+    import shutil
+    egg_name = "setuptools-%s-py%s.egg" % (version, sys.version[:3])
     url = download_base + egg_name
     saveto = os.path.join(to_dir, egg_name)
     src = dst = None
@@ -157,53 +172,24 @@
 and place it in this directory before rerunning this script.)
 ---------------------------------------------------------------------------""",
                     version, download_base, delay, url
-                ); from time import sleep; sleep(delay)
+                )
+                from time import sleep
+                sleep(delay)
             log.warn("Downloading %s", url)
             src = urllib2.urlopen(url)
             # Read/write all in one block, so we don't create a corrupt file
             # if the download is interrupted.
             data = _validate_md5(egg_name, src.read())
-            dst = open(saveto,"wb"); dst.write(data)
+            dst = open(saveto, "wb")
+            dst.write(data)
         finally:
-            if src: src.close()
-            if dst: dst.close()
+            if src:
+                src.close()
+            if dst:
+                dst.close()
     return os.path.realpath(saveto)
 
 
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
 def main(argv, version=DEFAULT_VERSION):
     """Install or upgrade setuptools and EasyInstall"""
     try:
@@ -212,9 +198,9 @@
         egg = None
         try:
             egg = download_setuptools(version, delay=0)
-            sys.path.insert(0,egg)
+            sys.path.insert(0, egg)
             from setuptools.command.easy_install import main
-            return main(list(argv)+[egg])   # we're done here
+            return main(list(argv) + [egg])   # we're done here
         finally:
             if egg and os.path.exists(egg):
                 os.unlink(egg)
@@ -226,7 +212,7 @@
             )
             sys.exit(2)
 
-    req = "setuptools>="+version
+    req = "setuptools>=" + version
     import pkg_resources
     try:
         pkg_resources.require(req)
@@ -235,16 +221,18 @@
             from setuptools.command.easy_install import main
         except ImportError:
             from easy_install import main
-        main(list(argv)+[download_setuptools(delay=0)])
-        sys.exit(0) # try to force an exit
+        main(list(argv) + [download_setuptools(delay=0)])
+        sys.exit(0)  # try to force an exit
     else:
         if argv:
             from setuptools.command.easy_install import main
             main(argv)
         else:
-            print "Setuptools version",version,"or greater has been installed."
+            print "Setuptools version", version, "or greater has been " \
+                + "installed."
             print '(Run "ez_setup.py -U setuptools" to reinstall or upgrade.)'
 
+
 def update_md5(filenames):
     """Update our built-in md5 registry"""
 
@@ -252,7 +240,7 @@
 
     for name in filenames:
         base = os.path.basename(name)
-        f = open(name,'rb')
+        f = open(name, 'rb')
         md5_data[base] = md5(f.read()).hexdigest()
         f.close()
 
@@ -262,7 +250,9 @@
 
     import inspect
     srcfile = inspect.getsourcefile(sys.modules[__name__])
-    f = open(srcfile, 'rb'); src = f.read(); f.close()
+    f = open(srcfile, 'rb')
+    src = f.read()
+    f.close()
 
     match = re.search("\nmd5_data = {\n([^}]+)}", src)
     if not match:
@@ -270,19 +260,13 @@
         sys.exit(2)
 
     src = src[:match.start(1)] + repl + src[match.end(1):]
-    f = open(srcfile,'w')
+    f = open(srcfile, 'w')
     f.write(src)
     f.close()
 
 
-if __name__=='__main__':
-    if len(sys.argv)>2 and sys.argv[1]=='--md5update':
+if __name__ == '__main__':
+    if len(sys.argv) > 2 and sys.argv[1] == '--md5update':
         update_md5(sys.argv[2:])
     else:
         main(sys.argv[1:])
-
-
-
-
-
-

=== modified file 'misc/bash-completion'
--- misc/bash-completion	2012-05-23 21:56:10 +0000
+++ misc/bash-completion	2013-02-25 21:57:30 +0000
@@ -49,10 +49,11 @@
         fi
     fi
 
-    cmdOpts=( )
-            cmdOpts=(--help)
-    optEnums=( )
-    fixedWords=( )
+    cmdOpts=()
+    cmdOpts=(--help)
+    optEnums=()
+    fixedWords=()
+
     case "$cmd" in
         get)
             for charm in $(_charm_official_charms) ; do
@@ -66,13 +67,13 @@
     IFS=$'\n'
     if [[ "$cur" == = ]] && [[ ${#optEnums[@]} -gt 0 ]]; then
         # complete directly after "--option=", list all enum values
-        COMPREPLY=( "${optEnums[@]}" )
+        COMPREPLY=("${optEnums[@]}")
         return 0
     else
-        fixedWords=( "${cmdOpts[@]}"
-                     "${globalOpts[@]}"
-                     "${optEnums[@]}"
-                     "${fixedWords[@]}" )
+        fixedWords=("${cmdOpts[@]}"
+                    "${globalOpts[@]}"
+                    "${optEnums[@]}"
+                    "${fixedWords[@]}")
     fi
 
     if [[ ${#fixedWords[@]} -gt 0 ]]; then

=== modified file 'scripts/create'
--- scripts/create	2012-08-28 18:59:42 +0000
+++ scripts/create	2013-02-25 21:57:30 +0000
@@ -18,7 +18,8 @@
 #    You should have received a copy of the GNU General Public License
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import sys, os
+import os
+import sys
 import os.path as path
 import time
 import shutil
@@ -31,15 +32,16 @@
 from Cheetah.Template import Template
 from stat import ST_MODE
 
+
 def portable_get_maintainer():
     """ Portable best effort to determine a maintainer """
     if 'NAME' in os.environ:
         name = os.environ['NAME']
     else:
-        name = pwd.getpwuid( os.getuid() ).pw_gecos.split(',')[0].strip()
+        name = pwd.getpwuid(os.getuid()).pw_gecos.split(',')[0].strip()
 
     if not len(name):
-        username = pwd.getpwuid( os.getuid() ) [0]
+        username = pwd.getpwuid(os.getuid())[0]
         name = username
 
     email = os.environ.get('EMAIL', '%s@%s' % (name, socket.getfqdn()))
@@ -52,62 +54,67 @@
 
 parser = argparse.ArgumentParser()
 parser.add_argument('charmname', help='Name of charm to create.')
-parser.add_argument('charmhome', nargs='?', 
+parser.add_argument('charmhome', nargs='?',
         help='Dir to create charm in. Defaults to CHARM_HOME env var or PWD')
 args = parser.parse_args()
 
 if args.charmhome:
     charm_home = args.charmhome
 else:
-    charm_home = os.getenv('CHARM_HOME','.')
+    charm_home = os.getenv('CHARM_HOME', '.')
 
 home = path.abspath(path.join(path.dirname(sys.argv[0]), '..'))
 template_dir = path.join(home, 'templates')
 output_dir = path.join(charm_home, args.charmname)
-print "Generating template for " + args.charmname + " from templates in " + template_dir
+print "Generating template for " + args.charmname + " from templates in " \
+    + template_dir
 print "Charm will be stored in " + output_dir
 
 if path.exists(output_dir):
-  print output_dir + " exists. Please move it out of the way."
-  sys.exit(1)
-
-shutil.copytree(path.join(template_dir,'charm'), output_dir)
-
-v={'package': args.charmname,
-   'maintainer': '%s <%s>' % get_maintainer() }
+    print output_dir + " exists. Please move it out of the way."
+    sys.exit(1)
+
+shutil.copytree(path.join(template_dir, 'charm'), output_dir)
+
+v = {'package': args.charmname,
+   'maintainer': '%s <%s>' % get_maintainer()}
 
 try:
     c = apt.Cache()
     c.open()
     p = c[args.charmname]
-    print "Found " + args.charmname + " package in apt cache, as a result charm contents have been pre-populated based on package metadata."
+    print "Found " + args.charmname + " package in apt cache, as a result " \
+        + "charm contents have been pre-populated based on package metadata."
+
     v['summary'] = p.summary
-    v['description'] = textwrap.fill(p.description,width=72, subsequent_indent='  ')
+    v['description'] = textwrap.fill(p.description, width=72,
+                                     subsequent_indent='  ')
 except:
-    print "Failed to find " + args.charmname + " in apt cache, creating an empty charm instead."
+    print "Failed to find " + args.charmname + " in apt cache, creating " \
+        + "an empty charm instead."
     v['summary'] = '<Fill in summary here>'
     v['description'] = '<Multi-line description here>'
 
 for root, dirs, files in os.walk(output_dir):
-  for outfile in files:
-    full_outfile=path.join(root,outfile)
-    mode = os.stat(full_outfile)[ST_MODE]
-    try:
-      t = Template(file=full_outfile,searchList=(v))
-      o = tempfile.NamedTemporaryFile(dir=root,delete=False)
-      os.chmod(o.name, mode)
-      o.write(str(t))
-      o.close()
-      backupname = full_outfile + str(time.time())
-      os.rename(full_outfile, backupname)
-      try:
-        os.rename(o.name, full_outfile)
-        os.unlink(backupname)
-      except Exception, e:
-        print "WARNING: Could not enable templated file: " + str(e)
-        os.rename(backupname, full_outfile)
-        raise
-    except Exception, e:
-      print "WARNING: could not process template for " + full_outfile + ": " + str(e)
-      raise
-
+    for outfile in files:
+        full_outfile = path.join(root, outfile)
+        mode = os.stat(full_outfile)[ST_MODE]
+        try:
+            t = Template(file=full_outfile, searchList=(v))
+            o = tempfile.NamedTemporaryFile(dir=root, delete=False)
+            os.chmod(o.name, mode)
+            o.write(str(t))
+            o.close()
+            backupname = full_outfile + str(time.time())
+            os.rename(full_outfile, backupname)
+            try:
+                os.rename(o.name, full_outfile)
+                os.unlink(backupname)
+            except Exception, e:
+                print "WARNING: Could not enable templated file: " + str(e)
+                os.rename(backupname, full_outfile)
+                raise
+        except Exception, e:
+            print "WARNING: could not process template for " + full_outfile \
+                + ": " + str(e)
+            raise

=== modified file 'scripts/getall'
--- scripts/getall	2012-08-28 18:59:42 +0000
+++ scripts/getall	2013-02-25 21:57:30 +0000
@@ -1,8 +1,7 @@
 #!/bin/sh
 set -e
 
-usage()
-{
+usage() {
     echo "usage: getall path_to_charms" >&2
     echo "retrieves all charms from launchpad using mr and bzr"
     exit $1
@@ -21,12 +20,12 @@
 home=`readlink -f $home`
 
 if ! [ -d "$charm_home/.bzr" ] ; then
-  echo initializing $charm_home as bzr repository
-  bzr init-repo $charm_home
-  $home/update $charm_home
-  echo checking out all charms
-  mr -c $charm_home/.mrconfig -d $charm_home --trust-all checkout
-  echo "In order to update charms, run this script again."
+    echo initializing $charm_home as bzr repository
+    bzr init-repo $charm_home
+    $home/update $charm_home
+    echo checking out all charms
+    mr -c $charm_home/.mrconfig -d $charm_home --trust-all checkout
+    echo "In order to update charms, run this script again."
 else
-  mr -c $charm_home/.mrconfig -d $charm_home --trust-all update
+    mr -c $charm_home/.mrconfig -d $charm_home --trust-all update
 fi

=== modified file 'scripts/promulgate'
--- scripts/promulgate	2012-08-28 18:59:42 +0000
+++ scripts/promulgate	2013-02-25 21:57:30 +0000
@@ -66,7 +66,8 @@
         help='Increase verbosity level.')
 
     parser.add_option(
-        '-u', '--unpromulgate', dest='unpromulgate', action='store_true', default=False,
+        '-u', '--unpromulgate', dest='unpromulgate', action='store_true',
+        default=False,
         help='Un-promulgate this branch instead of promulgating it')
 
     parser.add_option(
@@ -74,12 +75,15 @@
         help='Override warnings and errors.  USE WITH EXTREME CARE !!!!')
 
     parser.add_option(
-        '-w', '--ignore-warnings', dest='ignore_warnings', action='store_true', default=False,
+        '-w', '--ignore-warnings', dest='ignore_warnings', action='store_true',
+        default=False,
         help='Promulgate this branch even with warnings from charm proof')
 
     parser.add_option(
-        '-o', '--owner-branch', dest='promulgate_owner_branch', action='store_true', default=False,
-        help='Promulgate a branch owned by a someone/group other than ~charmers')
+        '-o', '--owner-branch', dest='promulgate_owner_branch',
+        action='store_true', default=False,
+        help='Promulgate a branch owned by a someone/group other than '
+            + '~charmers')
 
     return parser.parse_args()
 
@@ -103,7 +107,8 @@
             sys.exit(1)
     if charm_proof == 100:
         if ignore_warnings:
-            logging.info("ignore-warnings enabled ... Continuing with warnings")
+            logging.info("ignore-warnings enabled ... Continuing with "
+                + "warnings")
         elif force:
             logging.info("force option enabled ... Continuing with warnings")
         else:
@@ -116,14 +121,14 @@
     charm_metadata = os.path.join(charm_dir, 'metadata.yaml')
     if not os.access(charm_metadata, os.R_OK):
         logging.error("can't read charm metadata: %s", charm_metadata)
-    
+
     with open(charm_metadata) as metadata:
         charm = yaml.load(metadata)
 
     return charm['name']
 
 
-def find_branch_to_promulgate(lp,charm_dir,branch_url):
+def find_branch_to_promulgate(lp, charm_dir, branch_url):
     if branch_url is None:
         tree, branch, relpath = bzrdir.BzrDir.open_containing_tree_or_branch(
             charm_dir)
@@ -143,7 +148,7 @@
     return charm_branch
 
 
-def get_lp_charm_series(lp,series):
+def get_lp_charm_series(lp, series):
     charm_distro = lp.distributions[DISTRIBUTION]
     if series is None:
         charm_series = charm_distro.current_series
@@ -163,30 +168,33 @@
 
 
 def update_branch_info(charm_branch, branch_status, branch_reviewer):
-    logging.info( "Setting status of %s to %s", charm_branch.bzr_identity, branch_status)
+    logging.info("Setting status of %s to %s", charm_branch.bzr_identity,
+        branch_status)
     charm_branch.lifecycle_status = branch_status
 
-    logging.info( "Setting reviewer of %s to %s", charm_branch.bzr_identity, branch_reviewer)
+    logging.info("Setting reviewer of %s to %s", charm_branch.bzr_identity,
+        branch_reviewer)
     charm_branch.reviewer = branch_reviewer
 
     charm_branch.lp_save()
 
 
-def update_official_charm_branch(lp,series,charm_branch,charm_name):
-    charm_series = get_lp_charm_series(lp,series)
+def update_official_charm_branch(lp, series, charm_branch, charm_name):
+    charm_series = get_lp_charm_series(lp, series)
     lp_charm = charm_series.getSourcePackage(name=charm_name)
     if charm_branch:
-        logging.info( 'Setting %s as the official branch for %s',
+        logging.info('Setting %s as the official branch for %s',
                       charm_branch.bzr_identity,
                       lp_charm.name)
-        update_branch_info(charm_branch, OFFICIAL_BRANCH_STATUS, lp.people[REVIEW_TEAM_NAME])
+        update_branch_info(charm_branch, OFFICIAL_BRANCH_STATUS,
+            lp.people[REVIEW_TEAM_NAME])
     else:
         logging.info('Removing official branch for %s', lp_charm.name)
     lp_charm.setBranch(branch=charm_branch, pocket=OFFICIAL_BRANCH_POCKET)
 
 
 def branch_owner(bzr_branch):
-    lp_url = bzr_branch.bzr_identity #TODO this really sucks... better way?
+    lp_url = bzr_branch.bzr_identity  # TODO this really sucks... better way?
     return lp_url.lstrip('lp:').split('/')[0]
 
 
@@ -198,7 +206,8 @@
 
 def main(argv):
     options, args = parse_options()
-    logging.basicConfig(level=log_level(options.verbose), format='%(levelname)s:%(message)s')
+    logging.basicConfig(level=log_level(options.verbose),
+        format='%(levelname)s:%(message)s')
 
     if len(args):
         charm_dir = args[0]
@@ -212,23 +221,25 @@
 
     if options.unpromulgate:
         logging.info('unpromulgating...')
-        charm_branch = None # makes LP delete the source package.
+        charm_branch = None  # makes LP delete the source package.
     else:
         logging.info('promulgating...')
-        charm_branch = find_branch_to_promulgate(lp,charm_dir,options.branch)
+        charm_branch = find_branch_to_promulgate(lp, charm_dir, options.branch)
 
     if not is_valid_owner(charm_branch, options.promulgate_owner_branch):
         logging.error(" Invalid branch owner: %s", branch_owner(charm_branch))
-        logging.error(" Branch push location must be owned by '~charmers'\n" +
-                      "       use `bzr push --remember lp:~charmers/charms/<series>/<charm-name>/trunk`\n" +
-                      "       or override this behavior using the '--owner-branch' option")
+        logging.error(" Branch push location must be owned by '~charmers'\n"
+                    + "   use `bzr push --remember lp:~charmers/charms/"
+                    + "<series>/<charm-name>/trunk`\n   or override this "
+                    + "behavior using the '--owner-branch'"
+                    + " option")
         return 1
 
-    update_official_charm_branch(lp, options.series, charm_branch, charm_name_from_metadata(charm_dir))
+    update_official_charm_branch(lp, options.series, charm_branch,
+        charm_name_from_metadata(charm_dir))
 
     return 0
 
 
 if __name__ == '__main__':
     sys.exit(main(sys.argv))
-

=== modified file 'scripts/review'
--- scripts/review	2012-07-02 04:18:07 +0000
+++ scripts/review	2013-02-25 21:57:30 +0000
@@ -26,12 +26,13 @@
 from launchpadlib.launchpad import Launchpad
 
 
-
 def parse_options():
     parser = argparse.ArgumentParser(
-        description="Review a charm by adding a comment to the corresponding charm bug.  The review comment can be passed as a --message arg or via stdin")
+        description="Review a charm by adding a comment to the corresponding"
+            + " charm bug.  The review comment can be passed as a --message"
+            + " arg or via stdin")
 
-    parser.add_argument('bug_id', 
+    parser.add_argument('bug_id',
         help='The launchpad bug for the charm being reviewed.')
 
     parser.add_argument(
@@ -64,7 +65,7 @@
 def get_message_from_stdin():
     stream = sys.stdin
     text = stream.read()
-    sys.stdin = open('/dev/tty') # 'reset' stdin to prompt if necessary
+    sys.stdin = open('/dev/tty')  # 'reset' stdin to prompt if necessary
     return text
 
 
@@ -73,19 +74,21 @@
         return message
     else:
         return get_message_from_stdin()
-            
+
 
 def prompt_to_continue(bug_id):
     logging.debug("prompting")
-    ans = raw_input("Really add this comment to launchpad bug #%s? y/[n] " % bug_id)
+    ans = raw_input("Really add this comment to launchpad bug #%s? y/[n] "
+                    % bug_id)
     return ans.strip().lower().startswith('y')
 
 
 def main(argv):
     args = parse_options()
-    logging.basicConfig(level=log_level(args.verbose), format='%(levelname)s:%(message)s')
+    logging.basicConfig(level=log_level(args.verbose),
+        format='%(levelname)s:%(message)s')
 
-    review_message = get_message(args.message) # before connecting to lp
+    review_message = get_message(args.message)  # before connecting to lp
 
     logging.debug('login with %s launchpad:', args.lp_instance)
     lp = Launchpad.login_with('charm-pilot', args.lp_instance)
@@ -99,7 +102,8 @@
 
         if args.skip_prompt or prompt_to_continue(bug_id):
             logging.debug('adding comment')
-            bug.newMessage(content=review_message) #TODO check return or catch exception
+            # TODO check return or catch exception
+            bug.newMessage(content=review_message)
         else:
             logging.debug('not adding comment')
 
@@ -112,4 +116,3 @@
 
 if __name__ == '__main__':
     sys.exit(main(sys.argv))
-

=== modified file 'scripts/review-queue'
--- scripts/review-queue	2012-05-23 20:42:33 +0000
+++ scripts/review-queue	2013-02-25 21:57:30 +0000
@@ -6,7 +6,8 @@
 import itertools
 import argparse
 
-def calculate_age(from_date = None):
+
+def calculate_age(from_date=None):
     if not from_date:
         return None
 
@@ -74,68 +75,83 @@
 
 def charm_review_queue():
     print "Connecting to launchpad..."
-    lp = Launchpad.login_anonymously('charm-tools', 'production', version='devel', launchpadlib_dir='~/.cache/launchpadlib')
+    lp = Launchpad.login_anonymously('charm-tools', 'production',
+        version='devel', launchpadlib_dir='~/.cache/launchpadlib')
     charm = lp.distributions['charms']
     charmers = lp.people['charmers']
     charm_contributors = lp.people['charm-contributors']
-    
+
     print "Querying launchpad for bugs ..."
-    bugs = charm.searchTasks(tags=['new-formula', 'new-charm'], tags_combinator="Any", status=['New', 'Confirmed', 'Triaged', 'In Progress', 'Fix Committed'])
-    charmers_bugs = charmers.searchTasks(status=['New', 'Confirmed', 'Triaged', 'In Progress', 'Fix Committed'])
-    
+    bugs = charm.searchTasks(tags=['new-formula', 'new-charm'],
+        status=['New', 'Confirmed', 'Triaged', 'In Progress',
+            'Fix Committed'],
+        tags_combinator="Any")
+    charmers_bugs = charmers.searchTasks(
+        status=['New', 'Confirmed', 'Triaged', 'In Progress',
+            'Fix Committed'])
+
     print "Querying launchpad for proposals ..."
     proposals = charmers.getRequestedReviews(status="Needs review")
-    charm_contributors_proposals = charm_contributors.getRequestedReviews(status="Needs review")
-    
+    charm_contributors_proposals = charm_contributors.getRequestedReviews(
+        status="Needs review")
+
     print "Building review_queue ..."
     queue = list()
     max_summary_length = 50
-    
+
     # Bugs in charms distribution and charmers group
-    for bug in itertools.chain( bugs.entries, charmers_bugs.entries ):
+    for bug in itertools.chain(bugs.entries, charmers_bugs.entries):
         entry_summary = bug['title'].split('"')[1].strip()
-        entry_age = datetime.datetime.utcnow() - datetime.datetime.strptime(bug['date_created'].split('+')[0], "%Y-%m-%dT%H:%M:%S.%f")
+        bug_created = datetime.datetime.strptime(
+            bug['date_created'].split('+')[0], "%Y-%m-%dT%H:%M:%S.%f")
+        entry_age = datetime.datetime.utcnow() - bug_created
         entry = {
-        'date_created' : bug['date_created'].split("T")[0],
-        'age' : str(entry_age).split('.')[0],
-        'summary' : (entry_summary[:max_summary_length] + '...') if len(entry_summary) > max_summary_length else entry_summary,
-        'item' : bug['web_link'],
-        'status' : bug['status'],
+        'date_created': bug['date_created'].split("T")[0],
+        'age': str(entry_age).split('.')[0],
+        'summary': (entry_summary[:max_summary_length] + '...')
+            if len(entry_summary) > max_summary_length else entry_summary,
+        'item': bug['web_link'],
+        'status': bug['status'],
         }
         queue.append(entry)
-        
+
     # Merge proposals in charmers group
-    for proposal in itertools.chain(proposals.entries, charm_contributors_proposals.entries):
+    for proposal in itertools.chain(proposals.entries,
+                                    charm_contributors_proposals.entries):
         proposal_summary = proposal['description']
-        proposal_age = datetime.datetime.utcnow() - datetime.datetime.strptime(proposal['date_created'].split('+')[0], "%Y-%m-%dT%H:%M:%S.%f")
+        proposal_date_created = datetime.datetime.strptime(
+            proposal['date_created'].split('+')[0], "%Y-%m-%dT%H:%M:%S.%f")
+        proposal_age = datetime.datetime.utcnow() - proposal_date_created
         if proposal_summary is None:
             proposal_summary = "Proposal"
         entry = {
-        'date_created' : proposal['date_created'].split("T")[0],
-        'age' : str(proposal_age).split('.')[0],
-        'summary' : (proposal_summary[:max_summary_length] + '...') if len(proposal_summary) > max_summary_length else proposal_summary,
-        'item' : proposal['web_link'],
-        'status' : proposal['queue_status'],
+            'date_created': proposal['date_created'].split("T")[0],
+            'age': str(proposal_age).split('.')[0],
+            'summary': (proposal_summary[:max_summary_length] + '...')
+                if len(proposal_summary) > max_summary_length
+                else proposal_summary,
+            'item': proposal['web_link'],
+            'status': proposal['queue_status'],
         }
         queue.append(entry)
 
     return(sorted(queue, key=lambda k: k['date_created']))
 
+
 def main():
     parser = argparse.ArgumentParser(
             description="Shows items needing the attention of ~charmers")
     parser.parse_args()
     review_queue = charm_review_queue()
-    keys = ['date_created', 'age', 'summary', 'item', 'status' ]
+    keys = ['date_created', 'age', 'summary', 'item', 'status']
     headers = ['Date Created', 'Age', 'Summary', 'Item', 'Status']
     print "Queue length: %d" % len(review_queue)
-    if type(review_queue) == type([]) and len(review_queue) > 0:
-        print format_as_table(review_queue, 
-                              keys, 
-                              header = headers,
-                              sort_by_key = 'date_created',
-                              sort_order_reverse = False)
-
+    if isinstance(review_queue, list) and len(review_queue) > 0:
+        print format_as_table(review_queue,
+                              keys,
+                              header=headers,
+                              sort_by_key='date_created',
+                              sort_order_reverse=False)
 
 if __name__ == "__main__":
     main()

=== modified file 'scripts/subscribers'
--- scripts/subscribers	2012-08-01 04:06:41 +0000
+++ scripts/subscribers	2013-02-25 21:57:30 +0000
@@ -28,31 +28,35 @@
 from launchpadlib import launchpad
 
 parser = argparse.ArgumentParser(
-        description="""Prints out a report and optionally corrects found 
-        instances where maintainers of charms are not subscribed to bugs
-        on their charm in launchpad.
-        
-        Users will need all charms they are interested in checking locally
-        in the specified repository so that metadata.yaml can be inspected
-        to find the maintainer.""")
+    description="""Prints out a report and optionally corrects found
+    instances where maintainers of charms are not subscribed to bugs
+    on their charm in launchpad.
+
+    Users will need all charms they are interested in checking locally
+    in the specified repository so that metadata.yaml can be inspected
+    to find the maintainer.""")
 parser.add_argument('--subscribed', default=False, action='store_true',
-        help='Show maintainers who are properly subscribed instead of unsubscribed')
+    help='Show maintainers who are properly subscribed instead of '
+    + 'unsubscribed')
 parser.add_argument('--repository', default=None, type=str,
-        help='Repository to look for charms in. Defaults to $JUJU_REPOSITORY or getcwd')
+    help='Repository to look for charms in. Defaults to $JUJU_REPOSITORY '
+    + 'or getcwd')
 parser.add_argument('--quiet', default=False, action='store_true',
-        help='Hide everything except maintainer subscription lists.')
+    help='Hide everything except maintainer subscription lists.')
 parser.add_argument('--series', '-s', default=None,
-        help='Which series of the charm store to run against. Defaults to current dev focus')
+    help='Which series of the charm store to run against. Defaults to '
+    + 'current dev focus')
 parser.add_argument('--maintainer', default=None,
-        help='Limit output to this maintainer\'s charms only.')
+    help='Limit output to this maintainer\'s charms only.')
 parser.add_argument('--log-priority', default='WARNING')
 parser.add_argument('--launchpad-instance', default='production')
 parser.add_argument('--fix-unsubscribed', default=False, action='store_true',
-        help='Add a bug subscription for any unsubscribed maintainers. Requires --maintainer')
+    help='Add a bug subscription for any unsubscribed maintainers. Requires '
+    + '--maintainer')
 parser.add_argument('--force-fix-all', default=False, action='store_true',
-        help=argparse.SUPPRESS)
+    help=argparse.SUPPRESS)
 parser.add_argument('charms', default=[], nargs='*',
-        help='Charms to check for subscriptions')
+    help='Charms to check for subscriptions')
 
 args = parser.parse_args()
 
@@ -74,13 +78,15 @@
 else:
     log_prio = logging.DEBUG
 
-logging.basicConfig(format='%(asctime)s %(levelname)s %(message)s', level=log_prio)
+logging.basicConfig(format='%(asctime)s %(levelname)s %(message)s',
+    level=log_prio)
 
 if args.quiet:
     logging.disable(logging.WARNING)
 
 if args.maintainer is None and args.fix_unsubscribed:
-    warn('Running --fix-unsubscribed and without --maintainer is against policy.')
+    warn('Running --fix-unsubscribed and without --maintainer is against'
+        + ' policy.')
     if not args.force_fix_all:
         warn('Use --force-fix-all to override policy')
         sys.exit(1)
@@ -125,10 +131,12 @@
 
 for charm_name in charms:
     try:
-        with open('%s/%s/%s/metadata.yaml' % (repository, current_series, charm_name)) as mdata:
+        with open('%s/%s/%s/metadata.yaml' % (repository, current_series,
+                charm_name)) as mdata:
             mdata_parsed = yaml.safe_load(mdata)
     except IOError:
-        warn('%s/%s has no metadata in charm repo' % (current_series, charm_name))
+        warn('%s/%s has no metadata in charm repo' % (current_series,
+            charm_name))
         continue
     try:
         maintainers = mdata_parsed['maintainer']
@@ -141,11 +149,13 @@
 
     if args.maintainer is not None:
         if args.maintainer not in maintainers:
-            maints_by_email = [ m.split('<')[1].split('>')[0] for m in maintainers ]
+            maints_by_email = [m.split('<')[1].split('>')[0]
+                               for m in maintainers]
             if args.maintainer not in maints_by_email:
-                debug('%s not in maintainer list %s' % (args.maintainer, maintainers))
+                debug('%s not in maintainer list %s' % (args.maintainer,
+                    maintainers))
                 continue
-    
+
     for maintainer in maintainers:
         maint_email = maintainer.split('<')[1].split('>')[0]
         lp_maintainer = lp.people.getByEmail(email=maint_email)
@@ -167,5 +177,6 @@
             else:
                 print msg
             if args.fix_unsubscribed:
-                info('adding bug subscription to %s for %s' % (charm_name, maint_email))
+                info('adding bug subscription to %s for %s' % (charm_name,
+                    maint_email))
                 pkg.addBugSubscription(subscriber=lp_maintainer)

=== modified file 'tests/helpers/run_with_timeout.py'
--- tests/helpers/run_with_timeout.py	2012-03-12 13:06:08 +0000
+++ tests/helpers/run_with_timeout.py	2013-02-25 21:57:30 +0000
@@ -1,6 +1,8 @@
 #!/usr/bin/python
 
-import signal,os,sys
+import os
+import sys
+import signal
 
 signal.alarm(60)
-os.execv(sys.argv[1],sys.argv[1:])
+os.execv(sys.argv[1], sys.argv[1:])

=== modified file 'tests/mediawiki.sh'
--- tests/mediawiki.sh	2012-03-12 13:06:08 +0000
+++ tests/mediawiki.sh	2013-02-25 21:57:30 +0000
@@ -2,18 +2,18 @@
 set -e
 repository=$1
 if [ -z "$repository" ] ; then
-  echo "usage: $0 path/to/repository|teardown"
-  exit 1
+    echo "usage: $0 path/to/repository|teardown"
+    exit 1
 fi
 
 JUJU=`which juju`
 
 if [ "$repository" = "teardown" ] ; then
-  $JUJU destroy-service wiki-db || echo could not teardown wiki-db
-  $JUJU destroy-service demo-wiki || echo could not teardown demo-wiki
-  $JUJU destroy-service wiki-cache || echo could not teardown wiki-cache
-  $JUJU destroy-service wiki-balancer || echo could not teardown wiki-balancer
-  exit 0
+    $JUJU destroy-service wiki-db || echo could not teardown wiki-db
+    $JUJU destroy-service demo-wiki || echo could not teardown demo-wiki
+    $JUJU destroy-service wiki-cache || echo could not teardown wiki-cache
+    $JUJU destroy-service wiki-balancer || echo could not teardown wiki-balancer
+    exit 0
 fi
 
 $JUJU deploy --repository=$repository local:mysql wiki-db

=== modified file 'tests/wiki-slave.sh'
--- tests/wiki-slave.sh	2012-03-12 13:06:08 +0000
+++ tests/wiki-slave.sh	2013-02-25 21:57:30 +0000
@@ -2,17 +2,17 @@
 set -e
 repository=$1
 if [ -z "$repository" ] ; then
-  echo "usage: $0 path/to/repository | teardown"
-  exit 1
+    echo "usage: $0 path/to/repository | teardown"
+    exit 1
 fi
 
 JUJU=`which juju`
 
 if [ "$repository" = "teardown" ] ; then
-  $JUJU destroy-service master
-  $JUJU destroy-service slave
-  $JUJU destroy-service demowiki
-  exit 0
+    $JUJU destroy-service master
+    $JUJU destroy-service slave
+    $JUJU destroy-service demowiki
+    exit 0
 fi
 
 $JUJU deploy --repository=$repository local:mysql master