Review: Approve code
Hi Colin,
What an interesting branch! Well, not the branch per se but the whole
endeavor. I knew this work was coming but hadn't paid attention to the details
so it was a good read.
<supertrivial>
In lib/lp/archivepublisher/uefi.py the items in __all__ should be alphabetized.
</supertrivial>
The comment in uefi.py:
The filename should be something like:
<TYPE>_<VERSION>_<ARCH>.tar.gz
seems a bit breezy to me. When I first read it I thought the naming convention
was optional. Should the description really read "The filename must be of the
form:" ?
In fact, if the filename does not follow that pattern there could be problems
in setTargetDirectory. Perhaps there needs to be checks in there and a test to
exercise it.
On a related note, it looks like setTargetDirectory and getSeriesKey both embed
the same knowledge about the structure of the filename. A common method could
be used by both so that the filename parsing (simple as it is) isn't repeated.
I know it was not your doing, but I also know you share an aversion to poor
spelling, so could you fix the occurrences of 'wheter' in
lib/lp/soyuz/interfaces/queue.py (and perhaps the one other place it appears).
Thanks Colin.
--
https://code.launchpad.net/~cjwatson/launchpad/custom-uefi/+merge/111626
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
