From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id lgbEHIMfsWRVUCEAWB0awg (envelope-from ) for ; Fri, 14 Jul 2023 06:12:19 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=oQttvEHF; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 653FC1E0BE; Fri, 14 Jul 2023 06:12:19 -0400 (EDT) Received: from server2.sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 2ADCF1E00F for ; Fri, 14 Jul 2023 06:12:17 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2074B3858289 for ; Fri, 14 Jul 2023 10:12:15 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2074B3858289 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1689329535; bh=gg29dMbED+aRCzwCg7tfW00iqmThh5kyTnw3aaBDPpc=; h=Date:Subject:To:Cc:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=oQttvEHFsssWTNz/ZbpoEjrCdgNTEuRGdo5t4cNNnS5TvlVIe8LAFW9XnSWoDWf9m znAGahitCP8wcMlI9bARxtPrJMczrQqvzWrg+R7gbeQBkSo+GireZEOLzxXEenU+NO 3f5MKXOTvd1M36fJXEVyVq7H9F/eSytCu59YX9uM= Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 7265E3858CD1 for ; Fri, 14 Jul 2023 10:11:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7265E3858CD1 Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-82--yvb1adbNSWUhHc8SdWuAA-1; Fri, 14 Jul 2023 06:11:49 -0400 X-MC-Unique: -yvb1adbNSWUhHc8SdWuAA-1 Received: by mail-qv1-f70.google.com with SMTP id 6a1803df08f44-635e10763f3so13670366d6.1 for ; Fri, 14 Jul 2023 03:11:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689329509; x=1691921509; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=gg29dMbED+aRCzwCg7tfW00iqmThh5kyTnw3aaBDPpc=; b=b/vBjWQZ67Pc9f7OFsmThQi3Xg3QyBXBr5rpTxpClRJcVzkRo4iYtPWuXSMYaPBISW MH3+pdQ8dQPNDu0PlHc2TIZlQsPE4vOb76iLPROIb5YHlRSwC9EPXZPWJoXkYuLsIr09 aABinRLb+CLZK2FBkImCAPzxOVInIemggRxTRo2rdfI0RBhf2WitlQGSztZMXNbY3jzp mRdv+Xi1czIsWIwOs11VuGFAub5dCNs+SavHq8yM1m/5MYberiIjfCE6lldAvCFdyH5t cgattN9WdFEyiOXz5IkM5kHc50JD13R0oV+v1dJgb63EGbO6eTiaUzsJ5CMiab8V4Lsp HRoA== X-Gm-Message-State: ABy/qLZ4Tei6C0yroHK7j+fNDa1MgLEKispo0G5jjHJ1vHIOtyo5zYiJ 4nzkll0q/byVSuMb1KE65ELdOHdtZraAInuPcyWmg1PUiBxtJU89uBpyN58S/MXIAjnTMVHrs0z zh8+k6eJe1iIjhyIH4qaLUg== X-Received: by 2002:a05:6214:3a02:b0:636:abd2:3b57 with SMTP id nw2-20020a0562143a0200b00636abd23b57mr3156918qvb.20.1689329509100; Fri, 14 Jul 2023 03:11:49 -0700 (PDT) X-Google-Smtp-Source: APBJJlELhUTXyCv1FY0NRXvkUC9r8ml5iu8KX8mMxDTRmCffm9qMpCqp2RYUXrf2ydmvoI+62SA52A== X-Received: by 2002:a05:6214:3a02:b0:636:abd2:3b57 with SMTP id nw2-20020a0562143a0200b00636abd23b57mr3156904qvb.20.1689329508804; Fri, 14 Jul 2023 03:11:48 -0700 (PDT) Received: from [192.168.0.129] (ip-94-112-225-44.bb.vodafone.cz. [94.112.225.44]) by smtp.gmail.com with ESMTPSA id g7-20020ae9e107000000b0075cd80fde9esm3655128qkm.89.2023.07.14.03.11.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 14 Jul 2023 03:11:48 -0700 (PDT) Message-ID: <71c89a80-de73-6493-1ee4-f576ab0301d1@redhat.com> Date: Fri, 14 Jul 2023 12:11:45 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH v3 0/1] update MAINTAINERS file with git trailers To: Eli Zaretskii , Kevin Buettner Cc: gdb-patches@sourceware.org, pedro@palves.net, aburgess@redhat.com, brobecker@adacore.com, simon.marchi@polymtl.ca, tom@tromey.com, tdevries@suse.de, ulrich.weigand@de.ibm.com References: <20230713105651.2281574-2-blarsen@redhat.com> <20230713145047.358e2c4a@f37-zws-nv> <83h6q73uu3.fsf@gnu.org> In-Reply-To: <83h6q73uu3.fsf@gnu.org> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Bruno Larsen via Gdb-patches Reply-To: Bruno Larsen Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 14/07/2023 07:50, Eli Zaretskii wrote: >> Date: Thu, 13 Jul 2023 14:50:47 -0700 >> From: Kevin Buettner >> Cc: gdb-patches@sourceware.org, pedro@palves.net, aburgess@redhat.com, >> brobecker@adacore.com, simon.marchi@polymtl.ca, tom@tromey.com, >> tdevries@suse.de, ulrich.weigand@de.ibm.com, eliz@gnu.org >> >> On Thu, 13 Jul 2023 12:56:51 +0200 >> Bruno Larsen wrote: >> >>> Right now there is one big unanswered question: Should we have a >>> specific tag to explicitly signal when a patch has been partially >>> approved? Eli asked for it to avoid people mechanically reading tags >>> from thinking that a patch has been fully approved when it was only >>> partial. >> I don't think we need a tag for this. Since we review and/or approve >> patches via email, I think some additional text stating which portions >> were reviewed or approved is sufficient. >> >> Suppose I'm an area maintainer or a global maintainer who has confident >> knowledge of a particular area. I might then do something like this: >> >> For the mn10300 architecture portions: >> Approved-by: Kevin Buettner >> >> Only the Approved-by tag would be added to the git trailer, but it's >> clear to anyone involved in the approval process that I haven't >> approved the patch in its entirety, only certain parts. If I were to >> review the rest of the patch, but not approve it, I see nothing wrong >> with also saying: >> >> For everything else: >> Reviewed-by: Kevin Buettner >> >> I also see nothing wrong with qualifying the 'Reviewed-by' or >> 'Acked-by' tags. Yes, we might end up with a patchwork of reviews, >> but we might also get more people involved with the review process, >> which I think would be a good thing. >> >> If we really want to include the portions reviewed in the trailer, then >> I suggest extending the format of the trailer, perhaps like this: >> >> Approved-by: Kevin Buettner (mn10300 only) > The above will only work if everyone pays attention to those > qualifications. Moreover, in Real Life, the response doesn't include > just two such lines, it in many cases includes more text, and those > qualifications can easily "drown" in that. > > Also, we used to have a way of saying "Approved, if those few nits are > fixed", and the above either removes that possibility entirely, or > will make it harder to determine whether and which parts were > approved, and on what conditions. This is not something that could ever be dealt with in automation, unless we also wrote the nits in machine friendly ways. If we want to continue using this style, we need to have context dependent tags. Maybe I can add a line about it in the file? > > I still don't understand why we need the "partial-approved" facility > that uses Approved-by. How is it different from Reviewed-by? the > submitter still needs to figure out whether all the parts were okayed > or not, so the only aspect this changes is making it more complicated > for area maintainers to write these tags, because instead of just a > single Reviewed-by they need to choose among two tags. There are a few reasons I don't like using reviewed-by. Most importantly, if a patch touches 2 separate areas, both area maintainers might send an rb tag and the contributor will keep waiting for an ab tag that is unnecessary. Another reason is that it could confuse things further when someone who doesn't have any approval rights looks at a patch and says things look ok, they might assume this rb has more power than it has. The difference between the ambiguous review and the ambiguous approval is that the former relies on knowing who has what level of authority, while the latter relies on interpreting the email context. I do like Kevin's idea of extending the tag to be "Approved-By: name [(areas)]", where (areas) is used if the approval is partial. I think this could be explained easily enough in the file to avoid confusion. It would not be general speech context, it would be explicit in the tag itself. Do you think that this enough to not make the partial approval be missed? If not, I'm fine with us introducing Partially-Approved-By. -- Cheers, Bruno > > So my vote is for reserving Approved-by only to the cases where the > entire patch is approved. Alternatively, we could introduce an > additional tag, like Partially-approved-by or something. > > I guess my point is that this should be simple and ideally include > only fixed text, not some free-form text that could lead to > misunderstandings and misinterpretations. >