Review existing comments and add new ones

Also clarified a statement

whipper/command/cd.py

@@ -66,6 +66,7 @@ disc and track template are:
 class _CD(BaseCommand):
     eject = True
 
+    # XXX: Pylint, parameters differ from overridden 'add_arguments' method
     @staticmethod
     def add_arguments(parser):
         parser.add_argument('-R', '--release-id',
@@ -205,6 +206,7 @@ class Info(_CD):
 
     # Requires opts.device
 
+    # XXX: Pylint, parameters differ from overridden 'add_arguments' method
     def add_arguments(self):
         _CD.add_arguments(self.parser)
 
@@ -228,6 +230,7 @@ Log files will log the path to tracks relative to this directory.
     # Requires opts.record
     # Requires opts.device
 
+    # XXX: Pylint, parameters differ from overridden 'add_arguments' method
     def add_arguments(self):
         loggers = list(result.getLoggers())
         default_offset = None
@@ -246,7 +249,6 @@ Log files will log the path to tracks relative to this directory.
                                  default='whipper',
                                  help=("logger to use (choose from: '%s" %
                                        "', '".join(loggers) + "')"))
-        # FIXME: get from config
         self.parser.add_argument('-o', '--offset',
                                  action="store", dest="offset",
                                  default=default_offset,
@@ -415,6 +417,7 @@ Log files will log the path to tracks relative to this directory.
                                                   len(self.itable.tracks),
                                                   extra))
                         break
+                    # FIXME: catching too general exception (Exception)
                     except Exception as e:
                         logger.debug('got exception %r on try %d', e, tries)
 

whipper/command/main.py

@@ -45,6 +45,7 @@ def main():
         logger.critical("SystemError: %s", e)
         if (isinstance(e, common.EjectError) and
                 cmd.options.eject in ('failure', 'always')):
+            # XXX: Pylint, instance of 'SystemError' has no 'device' member
             eject_device(e.device)
         return 255
     except RuntimeError as e:

whipper/common/cache.py

@@ -104,8 +104,8 @@ class Persister:
         try:
             self.object = pickle.load(handle)
             logger.debug('loaded persisted object from %r', self._path)
+        # FIXME: catching too general exception (Exception)
         except Exception as e:
-            # TODO: restrict kind of caught exceptions?
             # can fail for various reasons; in that case, pretend we didn't
             # load it
             logger.debug(e)
@@ -127,7 +127,7 @@ class PersistedCache:
         try:
             os.makedirs(self.path)
         except OSError as e:
-            if e.errno != 17:  # FIXME
+            if e.errno != os.errno.EEXIST:  # FIXME: errno 17 is 'File Exists'
                 raise
 
     def _getPath(self, key):

whipper/common/program.py

@@ -297,6 +297,7 @@ class Program:
                 print('Type    : %s' % metadata.releaseType)
                 if metadata.barcode:
                     print("Barcode : %s" % metadata.barcode)
+                # TODO: Add test for non ASCII catalog numbers: see issue #215
                 if metadata.catalogNumber:
                     print("Cat no  : %s" %
                           metadata.catalogNumber.encode('utf-8'))

whipper/common/task.py

@@ -87,6 +87,7 @@ class PopenTask(task.Task):
                 return
 
             self._done()
+        # FIXME: catching too general exception (Exception)
         except Exception as e:
             logger.debug('exception during _read(): %s', e)
             self.setException(e)

whipper/extern/freedb.py

@@ -134,6 +134,7 @@ def perform_lookup(disc_id, freedb_server, freedb_port):
 
     if len(matches) > 0:
         # for each result, query FreeDB for XMCD file data
+        # XXX: Pylint, redefining argument with the local name 'disc_id'
         for (category, disc_id, _) in matches:
             sleep(1)  # add a slight delay to keep the server happy
 

whipper/extern/task/task.py

@@ -238,6 +238,7 @@ class Task(LogStub):
                 method = getattr(l, methodName)
                 try:
                     method(self, *args, **kwargs)
+                # FIXME: catching too general exception (Exception)
                 except Exception as e:
                     self.setException(e)
 
@@ -350,6 +351,7 @@ class BaseMultiTask(Task, ITaskListener):
             task.start(self.runner)
             self.debug('BaseMultiTask.next(): started task %d of %d: %r',
                        self._task, len(self.tasks), task)
+        # FIXME: catching too general exception (Exception)
         except Exception as e:
             self.setException(e)
             self.debug('Got exception during next: %r', self.exceptionMessage)
@@ -503,6 +505,7 @@ class SyncRunner(TaskRunner, ITaskListener):
         try:
             self.debug('start task %r' % task)
             task.start(self)
+        # FIXME: catching too general exception (Exception)
         except Exception as e:
             # getExceptionMessage uses global exception state that doesn't
             # hang around, so store the message

whipper/image/image.py

@@ -121,6 +121,7 @@ class ImageVerifyTask(task.MultiSeparateTask):
         task.MultiSeparateTask.__init__(self)
 
         self._image = image
+        # XXX: Pylint, redefining name 'cue' from outer scope (import)
         cue = image.cue
         self._tasks = []
         self.lengths = {}
@@ -183,6 +184,7 @@ class ImageEncodeTask(task.MultiSeparateTask):
         task.MultiSeparateTask.__init__(self)
 
         self._image = image
+        # XXX: Pylint, redefining name 'cue' from outer scope (import)
         cue = image.cue
         self._tasks = []
         self.lengths = {}

whipper/program/cdparanoia.py

@@ -538,6 +538,7 @@ class ReadVerifyTrackTask(task.MultiSeparateTask):
                     try:
                         logger.debug('moving to final path %r', self.path)
                         os.rename(self._tmppath, self.path)
+                    # FIXME: catching too general exception (Exception)
                     except Exception as e:
                         logger.debug('exception while moving to final '
                                      'path %r: %s', self.path, e)
@@ -546,6 +547,7 @@ class ReadVerifyTrackTask(task.MultiSeparateTask):
                     os.unlink(self._tmppath)
             else:
                 logger.debug('stop: exception %r', self.exception)
+        # FIXME: catching too general exception (Exception)
         except Exception as e:
             print('WARNING: unhandled exception %r' % (e, ))
 

whipper/result/logger.py

@@ -98,8 +98,6 @@ class WhipperLogger(result.Logger):
 
         # For every track include information in the TOC
         for t in table.tracks:
-            # FIXME: what happens to a track start over 60 minutes ?
-            # Answer: tested empirically, everything seems OK
             start = t.getIndex(1).absolute
             length = table.getTrackLength(t.number)
             end = table.getTrackEnd(t.number)

whipper/test/common.py

@@ -46,6 +46,7 @@ class TestCase(unittest.TestCase):
     # and we'd like to check for the actual exception under TaskException,
     # so override the way twisted.trial.unittest does, without failure
 
+    # XXX: Pylint, method could be a function (no-self-use)
     def failUnlessRaises(self, exception, f, *args, **kwargs):
         try:
             result = f(*args, **kwargs)