Code review comment for lp:~edwin-grubbs/launchpad/bug-568390-cyclic-membership-error

Revision history for this message
Paul Hummer (rockstar) wrote :

<rockstar> EdwinGrubbs, maybe validateOwner's abstract method should return False or raise NotImplementedError.
<EdwinGrubbs> rockstar: it's not required that validateOwner() be overridden. The vocabulary normally does a good enough job. This is the only subclass that actually does override it.
<EdwinGrubbs> so raising NotImplementedError isn't necessary, and returning false would be meaningless.
<rockstar> EdwinGrubbs, pass seems like it's very unopinionated
<EdwinGrubbs> rockstar: it's no different than LaunchpadFormView.validate().
<rockstar> EdwinGrubbs, okay.

review: Approve (code)

« Back to merge proposal