Code review comment for lp:~henninge/launchpad/bug-517080-pottery-split

Revision history for this message
Henning Eggers (henninge) wrote :

= Bug 517080 =

Although the bug talks about "splitting up", this branch really provides an extra function that simply checks for "POTFILES.in" existing in the tree. There was no code that could be re-used because the existing code works on the filesystem while this new function works directly on a bzrlib.Tree object.

== Implementation details ==

The code is straight forward - walk the tree and report success if a "POTFILES.in" is found.

This makes use of the new "with" statement with a context manager to acquire a lock on the tree. This seems to be a perfect use case for this new language feature and eventually the context manager should become part of bzrlib.Tree.

== Tests ==

The new test case uses the same packages as the tests that ran on the filesystem but uses BzrDir to create a working tree from the directories. The files in the trees don't actually get added and committed to the tree, simply because for the function to work this is not necessary.

To share some functionality between test cases, a mix-in class was created and some methods moved into it. This bloats the diff quite a bit because the method names lose a leading underscore.

bin/test -vvt pottery

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/translations/pottery/detect_intltool.py
  lib/lp/translations/tests/test_pottery_detect_intltool.py

« Back to merge proposal