Code review comment for lp:~blr/bzr/bug-1400567

Revision history for this message
Vincent Ladeuil (vila) wrote :

> ./bzr selftest -v patches.PatchesTester

Thanks so much for telling how to test the MP !

As a thank you hint: try './bzr selftest -v -s bt.test_patches' next time ;) The difference is that your command is less precise as it can match more tests, -s (--start-with) also avoid loading tests that won't be run

Overall, your approach is very clean and well tested.

I don't really like changing the return signature depending on an input parameter (especially for a generator) but I agree with your simpler approach, we can do refactoring later if needed (which is unclear) so that all call sites use your better returned values.

Thanks for the pep8 cleanups too which account for half of this proposal. Very much appreciated.

review: Approve

« Back to merge proposal