Open Stack

On 06/25/2014 03:56 AM, Martin Geisler wrote:
> Mark McLoughlin <markmc at redhat.com> writes:
> 
>> On Tue, 2014-06-24 at 13:56 -0700, Clint Byrum wrote:
>>> Excerpts from Mark McLoughlin's message of 2014-06-24 12:49:52 -0700:
>>>
>>> However, there is a debate, and thus I would _never_ block a patch
>>> based on this rule. It was feedback.. just as sometimes there is
>>> feedback in commit messages that isn't taken and doesn't lead to a
>>> -1.
>>
>> Absolutely, and I try and be clear about that with e.g. "not a -1" or
>> "if you're rebasing anyway, perhaps fix this".
> 
> Perhaps the problem is the round-trips such corrections imply?
> 
> In the Mercurial project we accept contributions sent as patches only.
> There it's common for the core developers to fix the commit message
> locally before importing a patch. That makes it quick to fix these
> problems and I think that this workflow puts less work on the core
> maintainers.
> 
> With Gerrit, it seems that simply fixing the commit message in the web
> interface could work. I know that a patch submitter can update it
> online, but I don't know if (core) reviewers can also just update it?

Anyone can actually upload a 2nd patch, which includes changing the
commit message. We just mostly have a culture of not rewriting people's
patches, for better or worse.

> (Updating the patch in Gerrit would "go behind the back" of the
> submitter who would then have to rebase any additional work he has done
> on the branch. So this is not 100% pain free.)

That's often the challenge, it works fine if the original author is
actually paying attention, and does a git review -d instead of just
using their local branch. But is many cases that's not happening. (Also
it's completely off book for how we teach folks to use git --amend in
the base case).

I've had instances of working with someone where even though we were
talking on IRC during the whole thing, they kept overwriting the fix I
was sticking in for them to get the test fixed. So typically you only
want to do this with really advanced developers, with heads up that you
pushed over them.

Maybe there are trickier things we could do in git-review for this. But
it definitely gets goofy if you aren't paying attention.

I do also think people often get grumpy about other people rewriting
their code. Which I think is just human, so erring on the side of giving
feedback instead of taking it over is I think the right thing to do.

	-Sean


-- 
Sean Dague
http://dague.net

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 482 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140625/67970e2b/attachment.pgp>