Code review comment for lp:~oddbloke/pqm/merge-directives
Revision history for this message
| Robert Collins (lifeless) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
f33e652
(Get the code!)
On Sun, 2008-08-24 at 03:29 +0000, Daniel Watkins wrote:
> To address Robert's issues:
>
> > - uses more shell tests rather than python unit tests
> I'm not sure what the Python tests are missing. If you could give me some suggestions, I'd be happy to put them into practice.
Well, there are new shell tests; either they are redundant (in which
case, remove em :)) or they test something not tested in the python
tests (in which case move them into python tests).
> > - the new Command class won't render correctly in the web GUI
> Fixed.
>
> > - I don't like the merge_callback lambda usage, its unobvious and doesn't seem necessary.
> I agree. Fixed. In the process, replay merge became its own Command (rather than being a part of CommandRunner) to stop my head exploding because of the broken merge abstraction we had.
Cool - thanks.
-Rob
--
GPG key available at: <http://