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 136E91F4C0 for ; Fri, 25 Oct 2019 16:10:40 +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:date:from:to:cc:subject:in-reply-to:message-id :references:mime-version:content-type; q=dns; s=default; b=XnUmk yVQgbZflso9rEXFBqGZqCnhw+OlWRXRpt/4O28OEuywX2aEVs9elPobPOXanrpuQ b2NO2PlcfFzj1Bmh6k1xQnqfifpLSficNPfz0nIHRF5Ui211IbZwGpEivd+O4hBw 8YJWHtmDIz+mMflKv65wPhWYEemSMJSYksD7mU= 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:date:from:to:cc:subject:in-reply-to:message-id :references:mime-version:content-type; s=default; bh=KfD9zMm/Jo8 gUyD7l3Qc04A2eLM=; b=yZNSgUnUgyx+NP3sI3D6doABfvKuzNSN2DfjBIW06pp OGhT8UvZBkJnWGDoi+YbdQPrtPYLnPFdRALvQyZvgrFlDkfAiWDLpLKWtoUHrLTe f76XzwqfCOZiOYZcXn3walsv/6vAmqH3lnqrv0OBH8AxfjIKhfHNabuwNHAaWq3I = Received: (qmail 86163 invoked by alias); 25 Oct 2019 16:10:36 -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 86144 invoked by uid 89); 25 Oct 2019 16:10:36 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: esa2.mentor.iphmx.com IronPort-SDR: iup8162fvxasBcF1YWNx2POsbF98RzDV3WayaPAyEi8rwVdDi809dXhAiM9xeGn37C5DC7BbWj p4aibmNp/A3WSQvEjnMgphxqZpXXYPq5buSZ7VpHDMRRZu/+xopNRPeqXGu/Y2GpsdwnH/KLpU p8yGs5f1NK0ie7pkNtHpuEAHOPk0d8+vQOkaGT5z7szGZtD3XhaeJPhXrmY+7xMiLtQ9JjvE32 nYqS1GQWNhEoNAas7Ru2UMDcciOAsag4p7C0dZAu5XetDQHfCGLWe81eFg+Dv6uZ/fBGsEr97X oSU= IronPort-SDR: E0j41AZJE6sR50M/7NfrtabUovSIVmDyC0447UtMooiJNlDFS9v5BesR+yvyGlvsh9eAdqNHhZ EsmdeX+pu770djBxC19p7eHAnDp1+pjiVK8Ch7PdEVKaVncI9oCHqPoB1NP76drwTRHv78a4T7 r/BK5s2HLGHroBS+hP40ZVLafSS8hy7pzBtKZreTGiQlGGvzewYY8mbUlsexdwcwDOuf15OWTG K0FoV5JA5/5Wgjd9+hxOnX8qTZUi1m8PRjYw8H1hwLitQ+kdcfS2Sj+c2rJ9F9GqUOgRzINf6N hMQ= Date: Fri, 25 Oct 2019 16:10:27 +0000 From: Joseph Myers To: Simon Marchi CC: Sergio Durigan Junior , Carlos O'Donell , libc-alpha , Jonathan Nieder Subject: Re: Setup non-pushing gerrit instance for glibc. In-Reply-To: <0374dde9-0b49-0af4-c718-e894b59e680f@polymtl.ca> Message-ID: References: <2e93ece9-386b-c587-9355-33a4695a3f02@redhat.com> <8b64d48f-3fd6-fa06-5b33-8daa97661fef@redhat.com> <87tv7xvaw7.fsf@redhat.com> <0374dde9-0b49-0af4-c718-e894b59e680f@polymtl.ca> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" On Fri, 25 Oct 2019, Simon Marchi wrote: > > In that case it should at least show a reasonable amount of context, with > > some indication of what function it is in, like in the diff hunk header. > > From my experience, git diff doesn't really get the function name, it just > gives you the first preceding line that starts with a non-whitespace. It > often "breaks" when you have labels or preprocessor directives at column 0, > for example: Yes. It's a reasonable heuristic that works in many common cases (and can be configured with a diff attribute in .gitattributes and a corresponding diff..xfuncname setting in .git/config, if desired). Sometimes the default amount of context in a diff isn't enough either, and sometimes code is rearranged in a way that makes diff output unhelpful. I'd like reasonable heuristics to be used in the gerrit messages so that what they show is sufficient in most cases for it to be possible to fully follow and contribute to the discussion via email without needing to go to the website. Showing the diff hunk, with default settings (so the same logic as used by default by git diff to identify a function name) seems like a reasonable heuristic for that purpose. > > There is a difference of *intent* between changes that depend on one > > another and a patch series. A patch series is saying: > > > > * there is a common purpose motivating the patches; and > > In Gerrit, you'd probably use topics to indicate that many changes have > a common purpose (whether they are interdependent or not). > > https://gerrit-review.googlesource.com/Documentation/intro-user.html#topics If using a topic were to mark the patches as a patch series (including marking email subjects and threading them accordingly), that would be fine. I don't think it's necessary for simply pushing multiple changes at once to be the way you cause something to be handled as a patch series, if topics are the idiomatic concept in gerrit that corresponds to a patch series as generated by git-format-patch. > > There's also the variant of patch series that are explained in the cover > > letter (or in other text not intended for the final commit) as being split > > only for review purposes and intended to be squashed for commit, if the > > pieces most convenient for review don't form bisectable commits. I'm not > > sure how much any review system needs to know about the distinction > > between the two kinds of patch series if it's not pushing the commits > > itself. > > I don't think there would be a problem with that, except that the two emails > on the mailing would not be threaded together. More thoughts about that > below. Well, there would be issues if you wanted gerrit to push changes itself. And how does the "identify this as being the same change" system (for knowing when something has been committed) work if multiple separately reviewed patches are squashed into one commit? (Is the valid syntax for Change-Ids documented anywhere? In particular, is it OK to write a human-readable Change-Id oneself?) -- Joseph S. Myers joseph@codesourcery.com