Code review comment for lp:~rachidbm/ubuntu-l10n-tools/search

Hi Rachid,

Thanks a lot for the branch!

A couple of things I've noticed while reviewing it:

* On setup.py you add "print >> sys.stderr, 'To build ul10n-tools you need https://launchpad.net/python-distutils-extra : \nsudo apt-get install python-distutils-extra python-polib'". While it is true, I don't think it's the best way to check it here, as the exception only checks for python-distutils-extra and shouldn't have anything to do with python-polib, which is not a build dependency, but a runtime dependency. I think having the check on the file that imports polib should be enough

* In ul10n_tools/lp_get_templates/__init__.py you add "DISTRO_CODENAMES = ('hardy', 'intrepid', 'jaunty', 'karmic', 'lucid', 'maverick', 'natty')", which is correct, but we should probably be more dynamic and use launchpadlib to get the releases. Nothing to change here, just a mental note to myself that we should check out the capabilites of the Launchpad API concerning templates nowadays, to see if we can get rid of the screen-scraping :)

* This is optional but you are using correct constructs such as "Source package: %s" % (options.source_package), but I'd recommend using "Source package: {0}".format(options.source_package) (See the str.format method on http://docs.python.org/library/stdtypes.html#string-methods: "This method of string formatting is the new standard in Python 3.0, and should be preferred to the % formatting [...]")

* In the future, if you submit a branch for each feature (i.e. each change you make for a tool), it will make it much easier to review (no need to do it for this now, I'll just review the current branch).

* On ul10n_tools/lp_set_pot_priority/__init__.py and ul10n_tools/search/TranslationsSearch.py you are adding some code that is commented out. If it's not used, could you please remove it?

Do you think you could have a look at these points and re-submit the merge proposal before merging?

Thanks!