Code review comment for lp:~thumper/launchpad/package-branch-edit-owner

Hey Tim,

Weird edge case. Thanks for fixing it up.

 * I wish you didn't add to the doctest, and instead added some real unit tests. Don't feel obliged to do anything about this wish.

 * If there were a function like get_allowed_owners(branch, editor) (or a method branch.get_allowed_owners(editor)) that returned a list of potential owners, then this wouldn't be an edge case. I guess what I mean is, why did you decide to fix this at the view level rather than the model level?

 * The big block of code you add in the doctest could do with some basic prose explanation, e.g. "Create an official branch owned by ~ubuntu-branches, and an editor who has nothing to do with ~ubuntu-branches but is still allowed to edit the branch."

You only need to do something about the last point as far as I'm concerned. Would very much appreciate your thoughts on the first two though.

jml