Code review comment for lp:~mandel/ubuntuone-client/improve-fs-notifications-logging

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

This code will not be executed if something fails before it runs:

        # clean logger
        manager._wdm[0].log.removeHandler(self.memento)

should be:

        # clean logger
        self.addCleanup(manager._wdm[0].log.removeHandler, self.memento)

(you need to move it up right after the log adding)

* While you're at it, can you please change this call:

        # clean behind us by removing the file
        os.remove(file_name)

to be

        # clean behind us by removing the file
        self.addCleanup(os.remove, file_name)

(you need to move it up right after the file creation)

* The proper styling for this:

# a map of the actions to names so that we have better logs.
WINDOWS_ACTIONS_NAMES = {
  1: 'IN_CREATE',
  2: 'IN_DELETE',
  3: 'IN_MODIFY',
  4: 'IN_MOVED_FROM',
  5: 'IN_MOVED_TO'}

should be:

# a map of the actions to names so that we have better logs.
WINDOWS_ACTIONS_NAMES = {
    1: 'IN_CREATE',
    2: 'IN_DELETE',
    3: 'IN_MODIFY',
    4: 'IN_MOVED_FROM',
    5: 'IN_MOVED_TO',
}

You have my +1 with those changes!

review: Needs Fixing

« Back to merge proposal