launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Please also link bug 785679 to this MP, you've fixed it too since you've re-enabled the test.

This broadly looks okay, although I will admit to a large sense of fear and trepidation of this hitting production. You're absolutely right, though, the buildd-manager needs more tests and less magic.

I am wary of the amount of times you are calling transaction.commit() in this branch. Especially in tests, where you are calling into methods that could commit() first and avoid all that duplication.

I am holding off my vote for now, this branch requires some thought. And possibly a large amount of vodka.
-- 
https://code.launchpad.net/~jtv/launchpad/bug-717969/+merge/83022
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-717969 into lp:launchpad.