Launchpad itself

Poor support for merge proposals superseded by different branch

Bug #596726 reported by Andrew Bennetts
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Launchpad itself
Triaged
Low
Unassigned

Bug Description

A relatively frequent occurrence in bzr's patch review process is this:

 1. Alice proposes a patch
 2. people review it, some changes are requested
 3. Bob implements the changes.

Launchpad should allow Bob to mark Alice's proposal as Superseded by his, and have the review comments on Alice's proposal displayed as they would be if Alice has superseded her own proposal. (Bob may even choose to immediately set the new proposal as Approved or Queued, if he is confident all the review comments have been addressed safely.)

Instead, Bob has to deal with having two separate proposals active, or inaccurately mark Alice's original proposal as Approved, Rejected, or Work in Progress. Rejected is probably the least confusing, but is also the most rude.

This can happen because Alice doesn't have time to follow-up the original proposal quickly, or the patch pilot is simply trying to avoid roundtrips by implementing obviously good suggestions.

A related case is someone doing a rework of a rejected patch from someone else to take a different approach, but it is still based on the original branch, and the original review comments are still relevant.

An example that is current as I write this bug report is: <https://code.edge.launchpad.net/~lifeless/bzr/fix-terminal-spew/+merge/28024> and <https://code.edge.launchpad.net/~abentley/bzr/fix-terminal-spew/+merge/25233>.

Tags: lp-code
Revision history for this message
Aaron Bentley (abentley) wrote :

Bug #504369 discusses how we want to fix this.

Revision history for this message
Martin Pool (mbp) wrote :

This confused me a lot today in https://code.edge.launchpad.net/~lifeless/bzr/encoding-option/+merge/28126

bug 504639 may not be completely right because there are sometimes multiple alternative proposals. Perhaps you want a graph between mps, showing A is superseded by both B and C? Perhaps that is overkill.

Revision history for this message
Robert Collins (lifeless) wrote :

Resubmit is interesting, but really a bit confusing.

One of the behaviours here is that reviewers get a huge diff rather than an incremental one, and resubmit seems to do that - thats a bit unpleasant and jarring.

Revision history for this message
Aaron Bentley (abentley) wrote : Re: [Bug 596726] Re: Poor support for merge proposals superseded by different branch

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/22/2010 12:39 AM, Robert Collins wrote:
> Resubmit is interesting, but really a bit confusing.

Can you give us some specific advice on how to make it less confusing?

> One of the behaviours here is that reviewers get a huge diff rather than
> an incremental one, and resubmit seems to do that - thats a bit
> unpleasant and jarring.

I don't understand. The diffs produced by resubmit are no different
from the diffs that would be produced by updating the branches in the
original proposal.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkwgtrQACgkQ0F+nu1YWqI01/wCfbhRqXx2nqvl6Fbp0+HOEuplo
jQAAnAy6is/UVTGgtqloUMzuiFbIL/nq
=3tAZ
-----END PGP SIGNATURE-----

Aaron Bentley (abentley)
Changed in launchpad-code:
status: New → Triaged
importance: Undecided → Medium
Revision history for this message
Robert Collins (lifeless) wrote :

If Bob does 3 commits on top of Alices work, the new content is those three diffs, not the rest of Alices branch. That is a lot less to read in email than a full diff which would be created if Bob created a brand new proposal; if updating the branches in the original proposal would create the same email diff as creating a brand new proposal then its still suboptimal.

Using your bzr plugin to show the additional work Bob has done would be the ideal thing - still show a full diff in the web page, but don't /notify/ using a full diff.

Changed in launchpad:
importance: Medium → Low
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.