[MERGE] Initial support for marking bugs as fixed using bzr
aaron.bentley at utoronto.ca
Fri Apr 13 20:25:52 BST 2007
-----BEGIN PGP SIGNED MESSAGE-----
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
>> 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
> 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
> 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
> 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
> 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:
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
-----END PGP SIGNATURE-----
More information about the bazaar