This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: How to keep Reviewed-by lines in git commits with gerrit.
* Joseph Myers:
> On Tue, 12 Nov 2019, Carlos O'Donell wrote:
>
>> - Gerrit does not close the review but adds a new patchset version
>> because the commit message changed.
>
> I don't see this as a useful way for it to behave.
>
> By including a Change-Id (that matches the Change-Id of something tracked
> in gerrit) in a commit pushed to master, the committer is making an
> assertion that what they are pushing is the latest version of the tracked
> change, and (by pushing) that it needs no further review. On that basis,
> the review should automatically be closed, without needing to add any new
> patchset version.
I'm not yet decided here.
> This shouldn't just be about Reviewed-by. If someone says a patch is OK
> with a specific change made to it (whether to the code or to the commit
> message), it should be enough to make that change and retest and push to
> master, without needing to go through extra administration in a review
> system just because of that change.
If you want to use Gerrit as some sort of audit trail (and we will
eventually face attacks on the GNU toolchain sources; maybe it has
already happened and we just haven't noticed yet), then of course it's
necessary that any tiny change gets re-reviewed.
If on the other hand it is just an optional tool to help a contributor
to produce their commit in a collaborative fashion, then it's indeed
silly to ask for a re-review once the contributor is satisfied with
what they've got.
But as discussed, for Gerrit in push mote and as far as Reviewed-By:
lines are concerned, this should be a non-issue. It's a consequence
of our own limited use of Gerrit.
> *Reducing* the amount of administration required is a good thing (for
> example, if we can develop a way for a commit message to say that the
> commit fixes a given bug, and for that to result in the bug being marked
> RESOLVED / FIXED with target milestone set automatically). Increasing the
> amount of administration needed for a patch that's OK with changes doesn't
> seem a good idea to me; patch tracking systems should be reducing the work
> humans need to do rather than adding extra steps.
Fully agreed.