[MERGE] Initial support for marking bugs as fixed using bzr

[John Arbash Meinel]

Aaron Bentley wrote:
> James Westby wrote:
>> On (12/04/07 18:07), Aaron Bentley wrote:
>>> James, please don't force 'bundle' to omit revisions.  Just use bzr.dev
>>> (or a mirror) as your submit location.
>>>
>>> Omitting revisions breaks bundle's usefulness as a means of shipping
>>> revisions, because I can't use your bundle to apply your changes.  It
>>> also obscures the full effect of your bundle, since merging your bundle
>>> introduces ListOption.
>> You can use my bundle to apply my changes. It just requires you to have
>> a branch that has Jonathan's bundle already applied.
> 
> The fact that it requires another bundle is brokenness.  I should not
> need anything other than your bundle to merge your changes.  That is the
> whole point of bundles.  If you don't want to do that, use a patch.
> 

I don't agree 100%...

I think James did something very helpful (to me). Which is to give me
the patch to review with just his changes so I can evaluate whether his
changes are good.

We may disagree with one of the things part of his patch is based on,
and still agree with his patch. At the moment, we have a bit of
discrepancy between "I agree with this patch" and "I agree with the
final state that this patch would create when combined with all patches
it depends on".

I would like a way to separate them. One possibility would be to just
use "+1 (conditional)" and then include the message "I like this patch,
and as soon as we merge what it depends on, it can be accepted".


>> Also it doesn't obscure the effect of my bundle. My bundle has nothing
>> to do with ListOption. You can see what changes I want to make from the
>> diff in mine.
> 
> Your bundle requires jml's and jml's is not yet merged.  It is derived
> from jml's bundle, so it supersedes it.  Merging your bundle will apply
> James' changes, so deciding whether to apply your bundle must include
> deciding whether to apply jml's changes.
> 
> Therefore it's misleading not to include jml's changes in your diff.
> 
>> However I accept that this may not be the preferred way of working, but
>> I don't believe there is a policy on that yet. If we want to have one
>> then please say so and I will not do it again. Allow me to explain the
>> reasons why I chose to do it this way.
> 
> I hereby propose a policy that all merge requests include a patch that
> shows all the changes that would be made if the merge were performed.
> 
> Aaron

I disagree with blanket statements, because I don't think they always apply.

If I just reviewed a 5,000 line patch, and liked all of it but maybe 20
lines. In the next review I would rather see the 20-line diff, than to
have to wade through 5,000 lines again.

As another example, if someone posts an update to someone else's patch,
which doesn't depend on a contentious portion of another patch, it is
nice to say that this is good.

I think there are a few issues....

1) BB only shows you 1 diff. There are potentially many ways you would
like to look at the changes. (whole changes, step-by-step, just the last
step, etc). BB works really well for the common case of small changes
over time.

2) BB doesn't have the concept of "extends" without "supersedes". So
this patch builds on that one, but doesn't replace it. I don't know how
we could implement this, without building a language. So I don't think
it is a BB limitation as much as a "automated review" limitation.

3) I should have used +1 (conditional) to indicate the patch was good
and should be merged if its dependency was merged.


1) could be addressed either by asking users to supply 2 patches (one
complete, and one to focus on). Which might also mean BB should support
displaying multiple diffs. (But that opens up a can-of-worms if both are
bundles and point to different revision ids. "Which one are you wanting
to be merged again????") I think humans can sort this stuff out, but I
don't know where that would leave BB.

Another possibility would be to extend BB with some of the loggerhead or
webserve functionality. So that you could look at the patch as a branch,
and look back over the different revisions. (I couldn't find a way to
get loggerhead to let you diff a revision against anything but its
parent, but I'm guessing that is just UI issues).

(2) Is probably not solvable.

(3) I can do in the future. And we can use as recommended policy for
changes like this.


Actually, I noticed we have stuff like "BzrGivingBack" to help users
submit patches. But we don't really have a place where we explain what
the different votes mean. (I know I probably use +1(cond) when I should
use +0)

John
=:->