From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 732611F4B5 for ; Tue, 12 Nov 2019 22:36:03 +0000 (UTC) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:references:date:in-reply-to :message-id:mime-version:content-type; q=dns; s=default; b=Qf7kM c+CqUQjJWsiCzQRfQ8NH9dcCc/A1E4CfRU0iddDlgBAslEHQvl2VVGivaWiIMiOu g2WLtpf/UWsZkNKMgUOI8jJ2HTk+MHvC6J0t7W4uFU+sKpdeX7Nh6rQLbCYDWdFT SFKB9AOWpQuQcPdQ9sbn5iFWNcyZki6PIH7eS4= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:references:date:in-reply-to :message-id:mime-version:content-type; s=default; bh=9ZLpxGJ4oVB pozi0s32EkhpgOwg=; b=hC1qoR4tk0m2FSwKP75xKmUED6zHMa3WUYuwDqfuQnv 8CEPqXHpL2s4MCfK6sCO4EQtWpLBxehjo0C1SSQcuNi9qZSDLWvSeoG3wViRuwa7 gH903GOB1FVwk/0thAdN2oOJDA8EZlDRF4Bx9s8RBo4fFLh29bngPXgooRqTdcPU = Received: (qmail 62695 invoked by alias); 12 Nov 2019 22:36:01 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 62687 invoked by uid 89); 12 Nov 2019 22:36:00 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: albireo.enyo.de From: Florian Weimer To: Joseph Myers Cc: Carlos O'Donell , libc-alpha , Florian Weimer Subject: Re: How to keep Reviewed-by lines in git commits with gerrit. References: <7b4aa5d2-14e7-c0aa-a258-bbd60455fae5@redhat.com> <28848c99-55e0-cbda-9230-3a0c3c8257ae@redhat.com> Date: Tue, 12 Nov 2019 23:35:53 +0100 In-Reply-To: (Joseph Myers's message of "Tue, 12 Nov 2019 22:26:30 +0000") Message-ID: <87eeycbu46.fsf@mid.deneb.enyo.de> MIME-Version: 1.0 Content-Type: text/plain * 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.