Comment 11 for bug 373078
Revision history for this message
| Andrew Bennetts (spiv) wrote Re: no code review status for 'merge with some tweaks' | #11 |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
0e1f616
(Get the code!)
This is a regular irritation I'm encountering with reviews of patches for bzr. I've started saying directly "I'm voting Needs Fixing, but once you have addressed these issues please consider my vote upgraded to Approved" in the comments.
I see Aaron's remark that:
> Needs fixing is meant to mean "This is approved but I'd like you to fix X"
> Resubmit is meant to be used when the reviewer is withholding
> approval until changes are made.
But that isn't how Needs Fixing is being interpreted in practice for the bzr project (either by reviewers or reviewees). Is the experience in other projects different? I gather the idea is that the presence of "Resubmit" is meant to disambiguate the meaning of "Needs Fixing", but I think instead it just confuses.
I have fond memories of the word "tweak" from BundleBuggy, but I'd be about as happy for the vote to be called "Conditional Approve", and think reviewees would find it at least as clear as "tweak". (Conditionally Approved would make a reasonable status description too, as BundleBuggy showed). Either "Tweak" or "Conditional Approve" would be an improvement over the current ambiguity of "Needs Fixing".