From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.freebsd.org (mx2.freebsd.org [IPv6:2610:1c1:1:606c::19:2]) by sourceware.org (Postfix) with ESMTPS id 22C803858C54 for ; Tue, 28 Nov 2023 16:39:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 22C803858C54 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=FreeBSD.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=FreeBSD.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 22C803858C54 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=2610:1c1:1:606c::19:2 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1701189587; cv=pass; b=opLHA2vDqCNswOWJ3HQWDl42po4Ter7GlDvNJEHTd1huKXls40M/Y1M3LbisNN2sx6y/Bg9TWc7FWx74dxiV1SxWgiI3rn53PUwx5X39M5OyCPfICEy3PO8a1CB6AFisqTKUJg1qLZ3fgz0NZ+LoglwCW3nwougiJb9/uN1BKGQ= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1701189587; c=relaxed/simple; bh=yVIRjYY4LFBetrMAKZXf+Klaq/pwPOSD4j6p55yO0Sc=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=u4EDgQCzg+mgixXY9leJAVDuAtBKj9sElunB9GujymqDDahVbBrXSsxB3MZ2QVHT+rCbgz2vyFp6gt+dER2ZleoJ+Dpw5/mqi7DggYiD7fX89+LmDg0oLDWXJ40BGkxdrY9JRCwxtU8JaZYCCRcjNjORBccDX6A4HeCG3dXf49w= ARC-Authentication-Results: i=2; server2.sourceware.org Received: from mx1.freebsd.org (mx1.freebsd.org [96.47.72.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits)) (Client CN "mx1.freebsd.org", Issuer "R3" (verified OK)) by mx2.freebsd.org (Postfix) with ESMTPS id 4Sfp8G5w0Yz3FCs; Tue, 28 Nov 2023 16:39:42 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Sfp8G4tzQz3WlW; Tue, 28 Nov 2023 16:39:42 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1701189582; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=HB0NcEtMpHNIRHjHPFp8JSFVjMMBtpT9mpLK2XoLQ4U=; b=n07WOjhNmy37yGyES/5JKTqzBat46lORcnHZtpihgZSjwm0E49+bwmSq93tvu0PVZ98xBD vYi3nAM2zmg/yoXR6F5etxOVW5ponSpebnGMO4UXlAmZaFFt/GpYXwKlR8BlGiUySbLE8x 828SKuBViHV9hFca89G52ZCRQA10fVQimBYEp2gDvOTjJGDlIlG7E3YT5gjt3vTxUKqolT iRnYLazdOEPikwF9GpvdZAX2KwduTwmhP2UQ1LUkJP9uI/sRdSOnWMCxdyfEPwK2IovdH+ 4VtQn9/omu3N0+rvvqh6vHbAnA1OLJ/OeFEsuPkGF/wf2P9XVNeHk7/mS/mpdw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1701189582; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=HB0NcEtMpHNIRHjHPFp8JSFVjMMBtpT9mpLK2XoLQ4U=; b=nNRMR8TrYpt0P0gZXp9RHMikO3Ucmn/+mNeZ1I3IBYmHqcbhvEzB+rf2TPFucDcigLPb4P S6pOIWNYZANG4IMNKLkFIMtWr+UXghdi/Glple/D9kk9kb/ay3+7JctvYFxheQ32ykV6XQ MKRHS6MOHZs2JyYCVdhRGAWAfvhQpIWb1KaP+xYA3Tcfj1Gj8VzbejaJvG3GHwHJyN3atO NctfyiWkodcut4dDkexla6wLjvFzg0u8eIddevpjFo7er8kYD7XJ0SR8xyM0Ayf4tXO3qg laSZcx/rgmBdk8qFLdc2mDLzwQ49WqtBkLQKrMxAKDRBwep/aHOYKydGeoOosg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1701189582; a=rsa-sha256; cv=none; b=cevNavWkEUhdLaFz5kPR6Ui1+JuH16arsn1MLc4GxBJIWyLCHScAE+k/1YAZPDpjYHbwjs crnJlev2v7iEI0lutgewBgGrPGruWuwGl2qCF2bBWE4IFSAfKgU7wstHtNmQt3U+Clw1h8 q8/vG/zSHSLebvIeQKuWnDPfZZwiKZ8XkNbNmc+xn4zTfMjh3oiJwbasVf6r+C1jsO78H7 Yk4tMt/h/SKGcWJz/zmVyUwhcwwBGFC8GTJ9HAySshAAiImhlS/KmKXaldPcbD9eTxB5HR kvV1UfjdlGV0ZsNZuL+7//BiUfaKMRwPzCoaOT9zWC/chkVMY1rhNCVNlGBhuA== Received: from [IPV6:2601:648:8384:fd00:b9d4:6be:a338:f57a] (unknown [IPv6:2601:648:8384:fd00:b9d4:6be:a338:f57a]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4Sfp8G28KNz1PTq; Tue, 28 Nov 2023 16:39:42 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <95c3ece1-cd12-4485-8c36-7ac69aa317e6@FreeBSD.org> Date: Tue, 28 Nov 2023 08:39:40 -0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6] [gdb]: add git trailer information on gdb/MAINTAINERS Content-Language: en-US To: Guinevere Larsen , gdb-patches@sourceware.org References: <20231102135457.3663735-2-blarsen@redhat.com> From: John Baldwin In-Reply-To: <20231102135457.3663735-2-blarsen@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 11/2/23 6:54 AM, Guinevere Larsen wrote: > The project has been using Tested-By (tb), Reviewed-By (rb) and > Approved-By (ab) for some time, but there has been no information to be > found in the actual repository. This commit changes that by adding > information about all git trailers to the MAINTAINERS file, so that it > can be easily double-checked. Simply put, the trailers in use work as > follows: > > * Tested-by: The person tested the patch and it fixes the problem, or > introduces no regressions (or both). > * Acked-by: The general outline looks good, but the maintainer hasn't > looked at the code > * Reviewed-by: The code looks good, but the reviewer has not approved > the patch to go upstream > * Approved-by: The patch is ready to be pushed to master > > These last 3 trailers can also be restricted to one or more areas of GDB > by adding the areas in a comma separated list in parenthesis after the > trailers. > > Finally, for completeness sake, the trailers Co-Authored-By and Bug > were added, even though they have been in use for a long time already > > --- > gdb/MAINTAINERS | 93 ++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 85 insertions(+), 8 deletions(-) > > diff --git a/gdb/MAINTAINERS b/gdb/MAINTAINERS > index 9989956065e..e1bb437a675 100644 > --- a/gdb/MAINTAINERS > +++ b/gdb/MAINTAINERS > @@ -43,14 +43,9 @@ patch without review from another maintainer. This especially includes > patches which change internal interfaces (e.g. global functions, data > structures) or external interfaces (e.g. user, remote, MI, et cetera). > > -The term "review" is used in this file to describe several kinds of feedback > -from a maintainer: approval, rejection, and requests for changes or > -clarification with the intention of approving a revised version. Review is > -a privilege and/or responsibility of various positions among the GDB > -Maintainers. Of course, anyone - whether they hold a position but not the > -relevant one for a particular patch, or are just following along on the > -mailing lists for fun, or anything in between - may suggest changes or > -ask questions about a patch! > +The word "contributor" is used in this document to refer to any GDB > +developer listed above as well as folks who may have suggested some > +patches but aren't part of one of those categories for any reason. > > There's also a couple of other people who play special roles in the GDB > community, separately from the patch process: > @@ -78,6 +73,88 @@ consensus among the global maintainers and any other involved parties. > In cases where consensus can not be reached, the global maintainers may > ask the official FSF-appointed GDB maintainers for a final decision. > > +The term "review" is used in this file to describe several kinds of > +feedback from a maintainer: approval, rejection, and requests for changes > +or clarification with the intention of approving a revised version. > +Approval is a privilege and/or responsibility of various positions among > +the GDB Maintainers. Of course, anyone - whether they hold a position, but > +not the relevant one for a particular patch, or are just following along on > +the mailing lists for fun, or anything in between - may suggest changes, ask > +questions about a patch or say if they believe a patch is fit for upstreaming! > + > +To ensure that patches are only pushed when approved, and to properly credit > +the contributors who take the time to improve this project, the following > +trailers are used to identify who contributed and how. All patches pushed > +upstream should have at least one Approved-By trailers (with the exception of > +obvious patches, see below). The trailers (or tags) currently in use are: This next to last sentence doesn't match current practice where a maintainer pushes a self-approved change. That is, to date we haven't used any 'Approved-by: ' trailers and this would seem to mandate that? (Also, s/trailers/trailer/ in that sentence) Also, there is some inconsistency with "by" vs "By" in this change. For example, this paragraph uses 'Approved-By' but the description below uses 'Approved-by'. The manpages for git(1) (e.g. git-interpret-trailer(1)) tend to only capitalize the first word, e.g. 'Signed-off-by'. Our existing log messages are a bit all over the place currently, e.g. for Co-authored-by we have the following in master to date: 127 Co-Authored-By: 9 Co-Authored-by: 35 Co-authored-by: For both Approved-by and Reviewed-by the uppercase 'By' variants outnumber lowercase 'by' by a fair margin. > + - Tested-by: > + > + Used when a contributor has tested the patch and finds that it > + fixes the claimed problem. It may also be used to indicate that > + the contributor has performed regression testing. By itself, this > + tag says nothing about the quality of the fix implemented by the > + patch, nor the amount of testing that was actually performed. > + Usage: "Tested-By: Your Name " I would find it slightly easier to read with a blank line before each Usage line. > + > + - Acked-By: > + > + Used when a responsible or global maintaner has taken a superficial s/maintaner/maintainer/ > + look at a patch and agree with its direction, but has not done further s/agree/agrees/ > + review on the subject. > + This trailer can be specific to one or more areas of the project, as > + defined by the "Responsible maintainers" section of this file. If > + that is the case, the area(s) should be added at the end of the tag in > + parenthesis in a comma separated list. I think you want s/comma separated/comma-separated/ here and elsewhere. > + Usage: "Acked-By: Your Name (area1, area2)" > + > + - Reviewed-by: > + > + Used when a contributor has looked at the code and agrees with > + the changes, but either doesn't have the authority or doesn't > + feel comfortable approving the patch. > + This trailer can be specific to one or more areas of the project, as > + defined by the "Responsible maintainers" section of this file. If > + that is the case, the area(s) should be added at the end of the tag in > + parenthesis in a comma separated list. > + Usage: "Reviewed-By: Your Name (area1, area2)" > + > + - Approved-by: > + > + Used by responsible maintainers or global maintainers when a patch is > + ready to be upstreamed. If a patch requires multiple approvals, only > + the last reviewer should use this tag, making it obvious to the > + contributor that the patch is ready to be pushed. > + This trailer can be specific to one or more areas of the project, as > + defined by the "Responsible maintainers" section of this file. If > + that is the case, the area(s) should be added at the end of the tag in > + parenthesis in a comma separated list. Patches must have all areas > + approved before being pushed. If a patch has had some areas approved, > + it is recommended that the final approver makes it explicit that the > + patch is ready for pushing. > + Responsible, Global and Official FSF-appointed maintainers may approve > + their own patches, but it is recommended that they seek external approval > + before doing so. > + Usage: "Approved-By: Your Name " > + > + - Co-Authored-By: > + > + Used when the commit includes meaningful conrtibutions from multiple people. s/conrtibutions/contributions/ > + Usage: "Co-Authored-By: Contributor's Name " > + > + - Bug: > + > + This trailer is added with a link to the GDB bug tracker bug for > + added context on relevant commits. > + Usage: "Bug: " > + > +Sometimes, contributors may request small changes, such as fixing typos, before > +granting the review or approval trailer. When the contributor thinks that > +these changes are so small that it isn't necessary to send a new version, they > +may add some text like "with these changes, I'm ok with the patch", followed by > +their trailer. In those situations, the trailer is only valid after the > +changes are made. > + > > The Obvious Fix Rule > -------------------- -- John Baldwin