Comment 2 for bug 326056
Revision history for this message
| Tim Penhey (thumper) wrote Re: [Bug 326056] [NEW] OOPS on InvalidStateTransition when reviewingcode bymail | #2 |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
0e1f616
(Get the code!)
On Sat, 07 Feb 2009 04:44:44 Aaron Bentley wrote:
> Paul Hummer wrote:
> > So I did a code review for Entertainer and sent it, then immediately
> > merged the branch into trunk and pushed up to launchpad. The scanner
> > apparently marked the BMP as merged before my review with ' status
> > approved' could get handled. This resulted in this oops:
> > https:/
>
> It's possible for something to be merged that is not approved, and it's
> possible for something to be approved that isn't merged. So I think
> that our model is slightly wrong. The ideal model would have two status
> flags:
>
> code review status: inactive, pending, approved, disapproved
s/disapproved/
> merge status: inactive, queued, merged
Would we still want to somehow record merge_failed? As in we tried to merge
but failed due to either conflicts or test failures.
> If our model reflected this, then setting the review status to approved
> would not change the merge status, and we could not get errors like this.
Now that's a damn fine idea.