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

[Aaron Bentley]

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

John Arbash Meinel wrote:
> Aaron Bentley wrote:
>> James Westby wrote:
>> 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.

A patch would have served that purpose just as well.  A bundle that
can't be merged is pointless.

Look, his email starts with a description of problems with ListOption,
so I go to read about ListOption in the diff, and there's absolutely
nothing there about it.  That's very unhelpful to me.

> 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".

The old delta-versus-snapshot issue. :-)

> 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".

Yeah, we haven't had enough of these to warrant special handling yet, I
think.

>> 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.

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

I think that should be our policy, even if there are deviations, because
otherwise we can merge bad code by accident.

Say I had already test-merged jml's patch, and backed it out.  Now the
bundle's dependencies are in my repo.

Now I read John's bundle, don't see the connection to jml's, and approve
it.  I merge it, and commit it.

I have now committed code that I disapprove of.

> 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.

In those cases, we have previously provided a diff and a branch.  That
may be a case where we have to vary, but typically we are dealing with
well-trusted authors.

> 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.

In that case, I would like to see both the submitter's changes, and the
overall changes.

> 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".

The difference is extremely fuzzy in reality also.

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

That would help.

> 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.

In cases like this one, I think BB could pick the latest revision.
Also, merge directives are unambiguous about which revision they suggest
merging.

> 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 am hesitant to put heavy-duty VCS functions on that box.  It's only a
virtual server.

> 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)

Actually, we do:
http://bundlebuggy.aaronbentley.com/help

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

iD8DBQFGH9lA0F+nu1YWqI0RAtZ0AJ9W8RMRvIlNQ9i7CK79Vc/m6iwGAwCfdBSR
b+ab7qx90DMlvFQ8seUt6yM=
=0ib2
-----END PGP SIGNATURE-----