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.

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to