| Message ID | ff130918-cbbb-2afc-d752-1af2ca9df340@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] wwwdocs: e-mail subject lines for contributions | expand |
On Tue, Jan 21, 2020 at 02:52:00PM +0000, Richard Earnshaw (lists) wrote: > [updated, following some comments from Gerald, main differences are > slight tweaks to the html markup and changing "email" to "e-mail"] > > This patch proposes some new (additional) rules for email subject lines > when contributing to GCC. The goal is to make sure that, as far as > possible, the subject for a patch will form a good summary when the > message is committed to the repository if applied with 'git am'. Where > possible, I've tried to align these rules with those already in > use for glibc, so that the differences are minimal and only where > necessary. > > Some things that differ from existing practice (at least by some people) > are: > > - Use '<topic>:' rather than '[<topic>]' > - This is more git friendly and works with 'git am'. > - Put bug numbers at the end of the line rather than the beginning. > - The bug number is useful, but not as useful as the brief summary. > Also, use the shortened form, as the topic part is more usefully > conveyed in the proper topic field (see above). Some examples would be useful I'd say, e.g. it is unclear in what way you want the PR number to be appended, shall it be something: whatever words describe it PR12345 or something: whatever words describe it (PR12345) or something: whatever words describe it: PR12345 or something: whatever words describe it [PR12345] or something else? Also, it would be nice to stress that the PR long form should be in the ChangeLog and somewhere on the later lines of the commit message, I don't think we pick up the shorter form from the first line when it short form (I could be wrong, but e.g. https://gcc.gnu.org/g:865257c447cc50f5819e9b53da83145f3c36c305 commit didn't make it into bugzilla). Jakub
On 21/01/2020 15:04, Jakub Jelinek wrote: > On Tue, Jan 21, 2020 at 02:52:00PM +0000, Richard Earnshaw (lists) wrote: >> [updated, following some comments from Gerald, main differences are >> slight tweaks to the html markup and changing "email" to "e-mail"] >> >> This patch proposes some new (additional) rules for email subject lines >> when contributing to GCC. The goal is to make sure that, as far as >> possible, the subject for a patch will form a good summary when the >> message is committed to the repository if applied with 'git am'. Where >> possible, I've tried to align these rules with those already in >> use for glibc, so that the differences are minimal and only where >> necessary. >> >> Some things that differ from existing practice (at least by some people) >> are: >> >> - Use '<topic>:' rather than '[<topic>]' >> - This is more git friendly and works with 'git am'. >> - Put bug numbers at the end of the line rather than the beginning. >> - The bug number is useful, but not as useful as the brief summary. >> Also, use the shortened form, as the topic part is more usefully >> conveyed in the proper topic field (see above). > > Some examples would be useful I'd say, e.g. it is unclear in what way you > want the PR number to be appended, shall it be > something: whatever words describe it PR12345 > or > something: whatever words describe it (PR12345) > or > something: whatever words describe it: PR12345 > or > something: whatever words describe it [PR12345] > or something else? Glibc use "[BZ #nnnn]" - obviously BZ becomes PR, but after that, I'm not too worried. I'd be happy with [PR #nnnn], but if folk want something else, please say so quickly... I'll prepare an edit for that... > > Also, it would be nice to stress that the PR long form should be in the > ChangeLog and somewhere on the later lines of the commit message, I don't > think we pick up the shorter form from the first line when it short form (I > could be wrong, but e.g. > https://gcc.gnu.org/g:865257c447cc50f5819e9b53da83145f3c36c305 > commit didn't make it into bugzilla). > Yes, good point. Thanks. R.
On Tue, Jan 21, 2020 at 03:33:22PM +0000, Richard Earnshaw (lists) wrote: > > Some examples would be useful I'd say, e.g. it is unclear in what way you > > want the PR number to be appended, shall it be > > something: whatever words describe it PR12345 > > or > > something: whatever words describe it (PR12345) > > or > > something: whatever words describe it: PR12345 > > or > > something: whatever words describe it [PR12345] > > or something else? > > Glibc use "[BZ #nnnn]" - obviously BZ becomes PR, but after that, I'm not > too worried. I'd be happy with [PR #nnnn], but if folk want something else, > please say so quickly... [PR 12345] or [PR #12345] is bad, because the bugzilla won't underline it, it needs to be either PR12345 word, or PR component/12345 . Jakub
On 21/01/2020 15:39, Jakub Jelinek wrote: > On Tue, Jan 21, 2020 at 03:33:22PM +0000, Richard Earnshaw (lists) wrote: >>> Some examples would be useful I'd say, e.g. it is unclear in what way you >>> want the PR number to be appended, shall it be >>> something: whatever words describe it PR12345 >>> or >>> something: whatever words describe it (PR12345) >>> or >>> something: whatever words describe it: PR12345 >>> or >>> something: whatever words describe it [PR12345] >>> or something else? >> >> Glibc use "[BZ #nnnn]" - obviously BZ becomes PR, but after that, I'm not >> too worried. I'd be happy with [PR #nnnn], but if folk want something else, >> please say so quickly... > > [PR 12345] or [PR #12345] is bad, because the bugzilla won't underline it, > it needs to be either PR12345 word, or PR component/12345 . > > Jakub > ok, lets go with [PRnnnn] then. R.
On 1/21/20 10:40 AM, Richard Earnshaw (lists) wrote: > On 21/01/2020 15:39, Jakub Jelinek wrote: >> On Tue, Jan 21, 2020 at 03:33:22PM +0000, Richard Earnshaw (lists) wrote: >>>> Some examples would be useful I'd say, e.g. it is unclear in what >>>> way you >>>> want the PR number to be appended, shall it be >>>> something: whatever words describe it PR12345 >>>> or >>>> something: whatever words describe it (PR12345) >>>> or >>>> something: whatever words describe it: PR12345 >>>> or >>>> something: whatever words describe it [PR12345] >>>> or something else? >>> >>> Glibc use "[BZ #nnnn]" - obviously BZ becomes PR, but after that, I'm >>> not >>> too worried. I'd be happy with [PR #nnnn], but if folk want >>> something else, >>> please say so quickly... >> >> [PR 12345] or [PR #12345] is bad, because the bugzilla won't underline >> it, >> it needs to be either PR12345 word, or PR component/12345 . > > ok, lets go with [PRnnnn] then. Doesn't this use of [] have the same problem with git am? My summaries are often describing the bug I'm fixing, i.e. [PATCH] PR c++/91476 - anon-namespace reference temp clash between TUs. which is also the first line of my ChangeLog entry. I think you are proposing [COMMITTED] c++: Fix anon-namespace reference temp clash between TUs (PR91476) which can no longer be shared with the ChangeLog. Jason
On 21/01/2020 17:20, Jason Merrill wrote: > On 1/21/20 10:40 AM, Richard Earnshaw (lists) wrote: >> On 21/01/2020 15:39, Jakub Jelinek wrote: >>> On Tue, Jan 21, 2020 at 03:33:22PM +0000, Richard Earnshaw (lists) >>> wrote: >>>>> Some examples would be useful I'd say, e.g. it is unclear in what >>>>> way you >>>>> want the PR number to be appended, shall it be >>>>> something: whatever words describe it PR12345 >>>>> or >>>>> something: whatever words describe it (PR12345) >>>>> or >>>>> something: whatever words describe it: PR12345 >>>>> or >>>>> something: whatever words describe it [PR12345] >>>>> or something else? >>>> >>>> Glibc use "[BZ #nnnn]" - obviously BZ becomes PR, but after that, >>>> I'm not >>>> too worried. I'd be happy with [PR #nnnn], but if folk want >>>> something else, >>>> please say so quickly... >>> >>> [PR 12345] or [PR #12345] is bad, because the bugzilla won't >>> underline it, >>> it needs to be either PR12345 word, or PR component/12345 . >> >> ok, lets go with [PRnnnn] then. > > Doesn't this use of [] have the same problem with git am? No, because only 'leading' [] blocks are removed - git mailinfo --help > > My summaries are often describing the bug I'm fixing, i.e. > > [PATCH] PR c++/91476 - anon-namespace reference temp clash between TUs. > > which is also the first line of my ChangeLog entry. I think you are > proposing > > [COMMITTED] c++: Fix anon-namespace reference temp clash between TUs > (PR91476) > > which can no longer be shared with the ChangeLog. > I was trying to unify this with glibc. They specify the bug number at the end of the line. We can diverge if it's generally felt to be important, but details like this create needless friction for folk working in both communities. R.
Hi! Thanks for doing this. On Tue, Jan 21, 2020 at 02:52:00PM +0000, Richard Earnshaw (lists) wrote: > This patch proposes some new (additional) rules for email subject lines > when contributing to GCC. The goal is to make sure that, as far as > possible, the subject for a patch will form a good summary when the > message is committed to the repository if applied with 'git am'. > +<p>A high-quality e-mail subject line for a contribution contains the > +following elements:</p> > + <li>A brief summary</li> You could stress that this is the one thing that really matters. And it's not a summary, it's much too brief for that (at most ~50 chars), but yup it should be something that allows *a human* to identify what this is. Everything else is just convention. > +<p>A component tag is a short identifier that identifies the part of > +the compiler being modified. This highlights to the relevant > +maintainers that the patch may need their attention. Multiple > +components may be listed if necessary. Each component tag should be > +followed by a colon. Often people use aaa/bbb: if drilling down a bit that way helps keep the subject short (which is the *point* of all this: keep things better consumable for humans). > +<p>The brief summary encapsulates in a few words the intent of the > +change. For example: <code>cleanup check_field_decls</code>.</p> It should start with a capital though. "Clean up blablal". (So no dot to end the sentence, this isn't a sentence). A capital helps the reader to quickly identify what is what, separate fluff from the core parts. > +<p>Some large patch sets benefit from an introductory e-mail that > +provides more context for the patch series and describes how the > +patches have been broken up to provide for review. All non-trivial series, yeah. Maybe we should mention how v2 etc. of patch series should show what is changed? If there is a good standard practice for that at all :-) Segher
On Tue, Jan 21, 2020 at 06:50:13PM +0000, Richard Earnshaw (lists) wrote: > > Doesn't this use of [] have the same problem with git am? > > No, because only 'leading' [] blocks are removed - git mailinfo --help I've used openmp: Teach omp_code_to_statement about rest of OpenMP statements [PR93329] as the first line of a commit log and that [PR93329] part is completely gone from it. Jakub
On 22/01/2020 09:07, Jakub Jelinek wrote: > On Tue, Jan 21, 2020 at 06:50:13PM +0000, Richard Earnshaw (lists) wrote: >>> Doesn't this use of [] have the same problem with git am? >> >> No, because only 'leading' [] blocks are removed - git mailinfo --help > > I've used > openmp: Teach omp_code_to_statement about rest of OpenMP statements [PR93329] > as the first line of a commit log and that [PR93329] part is completely gone > from it. > > Jakub > How did you apply the patch? R.
On Wed, Jan 22, 2020 at 09:35:31AM +0000, Richard Earnshaw (lists) wrote: > On 22/01/2020 09:07, Jakub Jelinek wrote: > > On Tue, Jan 21, 2020 at 06:50:13PM +0000, Richard Earnshaw (lists) wrote: > > > > Doesn't this use of [] have the same problem with git am? > > > > > > No, because only 'leading' [] blocks are removed - git mailinfo --help > > > > I've used > > openmp: Teach omp_code_to_statement about rest of OpenMP statements [PR93329] > > as the first line of a commit log and that [PR93329] part is completely gone > > from it. > > How did you apply the patch? Just git commit -a and editing the log message. But I got the [PR.....] not eaten in another commit, so ignore the above for now (although I'm very much convinced it was in there when I wrote the commit log). Jakub
On 21/01/2020 19:26, Segher Boessenkool wrote: > Hi! > > Thanks for doing this. > > On Tue, Jan 21, 2020 at 02:52:00PM +0000, Richard Earnshaw (lists) wrote: >> This patch proposes some new (additional) rules for email subject lines >> when contributing to GCC. The goal is to make sure that, as far as >> possible, the subject for a patch will form a good summary when the >> message is committed to the repository if applied with 'git am'. > >> +<p>A high-quality e-mail subject line for a contribution contains the >> +following elements:</p> > >> + <li>A brief summary</li> > > You could stress that this is the one thing that really matters. And > it's not a summary, it's much too brief for that (at most ~50 chars), > but yup it should be something that allows *a human* to identify what > this is. > Well, it should all matter, or why are we requiring it? I'm happy to insert 'very' in front of 'brief' and to say elsewhere that the entire subject (less the leading [...] part, should rarely, if ever, go beyond 80 characters. > Everything else is just convention. > >> +<p>A component tag is a short identifier that identifies the part of >> +the compiler being modified. This highlights to the relevant >> +maintainers that the patch may need their attention. Multiple >> +components may be listed if necessary. Each component tag should be >> +followed by a colon. > > Often people use aaa/bbb: if drilling down a bit that way helps keep the > subject short (which is the *point* of all this: keep things better > consumable for humans). The aaa: bbb: is really for when aaa and bbb are independent parts of the compiler and potentially needs multiple reviewers. aaa/bbb is when bbb is strictly a sub-component of aaa (for example, arm/SVE: would be an SVE related issue in the arm backend). I'm certainly not against having that. > >> +<p>The brief summary encapsulates in a few words the intent of the >> +change. For example: <code>cleanup check_field_decls</code>.</p> > > It should start with a capital though. "Clean up blablal". (So no > dot to end the sentence, this isn't a sentence). A capital helps > the reader to quickly identify what is what, separate fluff from the > core parts. > There is a balance here to be made. I'm mindful of Gerald's concern that this is perhaps overly restrictive for new users already. I certainly think we should have a good house style, but getting too prescriptive just gets in the way of attracting good contributions. >> +<p>Some large patch sets benefit from an introductory e-mail that >> +provides more context for the patch series and describes how the >> +patches have been broken up to provide for review. > > All non-trivial series, yeah. > > Maybe we should mention how v2 etc. of patch series should show what is > changed? If there is a good standard practice for that at all :-) > Dunno. I think that belongs primarily in the v2 0/n mail. It's not, after all, something that needs to be kept once the patches are applied. R. > > Segher >
On Wed, Jan 22, 2020 at 10:00:00AM +0000, Richard Earnshaw (lists) wrote: > On 21/01/2020 19:26, Segher Boessenkool wrote: > >On Tue, Jan 21, 2020 at 02:52:00PM +0000, Richard Earnshaw (lists) wrote: > >>+ <li>A brief summary</li> > > > >You could stress that this is the one thing that really matters. And > >it's not a summary, it's much too brief for that (at most ~50 chars), > >but yup it should be something that allows *a human* to identify what > >this is. > > > > Well, it should all matter, or why are we requiring it? Yes. > I'm happy to insert 'very' in front of 'brief' and to say elsewhere that > the entire subject (less the leading [...] part, should rarely, if ever, > go beyond 80 characters. The usual recommendation is 50 chars. Which is just about right with most MUAs. > >Everything else is just convention. > > > >>+<p>A component tag is a short identifier that identifies the part of > >>+the compiler being modified. This highlights to the relevant > >>+maintainers that the patch may need their attention. Multiple > >>+components may be listed if necessary. Each component tag should be > >>+followed by a colon. > > > >Often people use aaa/bbb: if drilling down a bit that way helps keep the > >subject short (which is the *point* of all this: keep things better > >consumable for humans). > > The aaa: bbb: is really for when aaa and bbb are independent parts of > the compiler and potentially needs multiple reviewers. aaa/bbb is when > bbb is strictly a sub-component of aaa (for example, arm/SVE: would be > an SVE related issue in the arm backend). I'm certainly not against > having that. Excellent. > >>+<p>The brief summary encapsulates in a few words the intent of the > >>+change. For example: <code>cleanup check_field_decls</code>.</p> > > > >It should start with a capital though. "Clean up blablal". (So no > >dot to end the sentence, this isn't a sentence). A capital helps > >the reader to quickly identify what is what, separate fluff from the > >core parts. > > There is a balance here to be made. I'm mindful of Gerald's concern > that this is perhaps overly restrictive for new users already. I > certainly think we should have a good house style, but getting too > prescriptive just gets in the way of attracting good contributions. This is just basic email etiquette :-) But, it's very annoying when you look at badly formatted subjects, in "git log --oneline" for example, it is very obvious there (and long subjects are problematic there as well btw). Wrt balance: yes, that is one reason why I do not think we should require all the markup stuff. > >>+<p>Some large patch sets benefit from an introductory e-mail that > >>+provides more context for the patch series and describes how the > >>+patches have been broken up to provide for review. > > > >All non-trivial series, yeah. > > > >Maybe we should mention how v2 etc. of patch series should show what is > >changed? If there is a good standard practice for that at all :-) > > Dunno. I think that belongs primarily in the v2 0/n mail. It's not, > after all, something that needs to be kept once the patches are applied. Sure. Ah, we could recommend that then, to make it clear in the vM 0/N of some series which patches changed how. My point is that it is a waste of time to everyone if a reviewer has to drag through everything every time; this is double true with git, it is easy to send updated versions of patch sets often. Segher
"Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> writes: > On 21/01/2020 17:20, Jason Merrill wrote: >> On 1/21/20 10:40 AM, Richard Earnshaw (lists) wrote: >>> On 21/01/2020 15:39, Jakub Jelinek wrote: >>>> On Tue, Jan 21, 2020 at 03:33:22PM +0000, Richard Earnshaw (lists) >>>> wrote: >>>>>> Some examples would be useful I'd say, e.g. it is unclear in what >>>>>> way you >>>>>> want the PR number to be appended, shall it be >>>>>> something: whatever words describe it PR12345 >>>>>> or >>>>>> something: whatever words describe it (PR12345) >>>>>> or >>>>>> something: whatever words describe it: PR12345 >>>>>> or >>>>>> something: whatever words describe it [PR12345] >>>>>> or something else? >>>>> >>>>> Glibc use "[BZ #nnnn]" - obviously BZ becomes PR, but after that, >>>>> I'm not >>>>> too worried. I'd be happy with [PR #nnnn], but if folk want >>>>> something else, >>>>> please say so quickly... >>>> >>>> [PR 12345] or [PR #12345] is bad, because the bugzilla won't >>>> underline it, >>>> it needs to be either PR12345 word, or PR component/12345 . >>> >>> ok, lets go with [PRnnnn] then. >> >> Doesn't this use of [] have the same problem with git am? > > No, because only 'leading' [] blocks are removed - git mailinfo --help > >> >> My summaries are often describing the bug I'm fixing, i.e. >> >> [PATCH] PR c++/91476 - anon-namespace reference temp clash between TUs. >> >> which is also the first line of my ChangeLog entry. I think you are >> proposing >> >> [COMMITTED] c++: Fix anon-namespace reference temp clash between TUs >> (PR91476) >> >> which can no longer be shared with the ChangeLog. >> > > I was trying to unify this with glibc. They specify the bug number at > the end of the line. > > We can diverge if it's generally felt to be important, but details like > this create needless friction for folk working in both communities. +1 for "component: Summary [PRnnnnn]" FWIW. PR bz-component/nnnnn works well for C++. The problem is that so many other PRs come under tree-optimization and rtl-optimization, which eat up a lot of subject line characters without narrowing things down very much. "cselib: ... [PRnnnnn]" is both shorter and more descriptive than "PR rtl-optimization/nnnnn - ....", etc. Same idea for "PR target/nnnnn - ...": you then need to say which target you mean in the main summary, whereas "aarch64: .... [PRnnnnn]" makes it easier to keep the main summary short. Maybe that's just a problem with the bz classification though... Richard
On Wed, Jan 22, 2020 at 04:05:37PM +0000, Richard Sandiford wrote: > "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> writes: > > On 21/01/2020 17:20, Jason Merrill wrote: > >> On 1/21/20 10:40 AM, Richard Earnshaw (lists) wrote: > >>> On 21/01/2020 15:39, Jakub Jelinek wrote: > >>>> On Tue, Jan 21, 2020 at 03:33:22PM +0000, Richard Earnshaw (lists) > >>>> wrote: > >>>>>> Some examples would be useful I'd say, e.g. it is unclear in what > >>>>>> way you > >>>>>> want the PR number to be appended, shall it be > >>>>>> something: whatever words describe it PR12345 > >>>>>> or > >>>>>> something: whatever words describe it (PR12345) > >>>>>> or > >>>>>> something: whatever words describe it: PR12345 > >>>>>> or > >>>>>> something: whatever words describe it [PR12345] > >>>>>> or something else? > >>>>> > >>>>> Glibc use "[BZ #nnnn]" - obviously BZ becomes PR, but after that, > >>>>> I'm not > >>>>> too worried. I'd be happy with [PR #nnnn], but if folk want > >>>>> something else, > >>>>> please say so quickly... > >>>> > >>>> [PR 12345] or [PR #12345] is bad, because the bugzilla won't > >>>> underline it, > >>>> it needs to be either PR12345 word, or PR component/12345 . > >>> > >>> ok, lets go with [PRnnnn] then. > >> > >> Doesn't this use of [] have the same problem with git am? > > > > No, because only 'leading' [] blocks are removed - git mailinfo --help > > > >> > >> My summaries are often describing the bug I'm fixing, i.e. > >> > >> [PATCH] PR c++/91476 - anon-namespace reference temp clash between TUs. > >> > >> which is also the first line of my ChangeLog entry. I think you are > >> proposing > >> > >> [COMMITTED] c++: Fix anon-namespace reference temp clash between TUs > >> (PR91476) > >> > >> which can no longer be shared with the ChangeLog. > >> > > > > I was trying to unify this with glibc. They specify the bug number at > > the end of the line. > > > > We can diverge if it's generally felt to be important, but details like > > this create needless friction for folk working in both communities. > > +1 for "component: Summary [PRnnnnn]" FWIW. > > PR bz-component/nnnnn works well for C++. The problem is that so many > other PRs come under tree-optimization and rtl-optimization, which > eat up a lot of subject line characters without narrowing things down > very much. "cselib: ... [PRnnnnn]" is both shorter and more descriptive > than "PR rtl-optimization/nnnnn - ....", etc. Yeah, the cselib version definitely looks preferable to me. What if a patch touches more than just the C++ FE, do we want "c,c++:"? Though kernel uses net: sched: act_ctinfo: fix memory leak locking/rwsem: Fix kernel crash when spinning on RWSEM_OWNER_UNKNOWN If a patch touches various spots in the optimizers, maybe we can just go with "tree-opt:" or "rtl:"? Further, I suppose multiple PRs fixed by a single patch would look like: c++: Implement DR 666 [PR57, PR12345] Marek
On 22/01/2020 16:28, Marek Polacek wrote: > On Wed, Jan 22, 2020 at 04:05:37PM +0000, Richard Sandiford wrote: >> "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> writes: >>> On 21/01/2020 17:20, Jason Merrill wrote: >>>> On 1/21/20 10:40 AM, Richard Earnshaw (lists) wrote: >>>>> On 21/01/2020 15:39, Jakub Jelinek wrote: >>>>>> On Tue, Jan 21, 2020 at 03:33:22PM +0000, Richard Earnshaw (lists) >>>>>> wrote: >>>>>>>> Some examples would be useful I'd say, e.g. it is unclear in what >>>>>>>> way you >>>>>>>> want the PR number to be appended, shall it be >>>>>>>> something: whatever words describe it PR12345 >>>>>>>> or >>>>>>>> something: whatever words describe it (PR12345) >>>>>>>> or >>>>>>>> something: whatever words describe it: PR12345 >>>>>>>> or >>>>>>>> something: whatever words describe it [PR12345] >>>>>>>> or something else? >>>>>>> >>>>>>> Glibc use "[BZ #nnnn]" - obviously BZ becomes PR, but after that, >>>>>>> I'm not >>>>>>> too worried. I'd be happy with [PR #nnnn], but if folk want >>>>>>> something else, >>>>>>> please say so quickly... >>>>>> >>>>>> [PR 12345] or [PR #12345] is bad, because the bugzilla won't >>>>>> underline it, >>>>>> it needs to be either PR12345 word, or PR component/12345 . >>>>> >>>>> ok, lets go with [PRnnnn] then. >>>> >>>> Doesn't this use of [] have the same problem with git am? >>> >>> No, because only 'leading' [] blocks are removed - git mailinfo --help >>> >>>> >>>> My summaries are often describing the bug I'm fixing, i.e. >>>> >>>> [PATCH] PR c++/91476 - anon-namespace reference temp clash between TUs. >>>> >>>> which is also the first line of my ChangeLog entry. I think you are >>>> proposing >>>> >>>> [COMMITTED] c++: Fix anon-namespace reference temp clash between TUs >>>> (PR91476) >>>> >>>> which can no longer be shared with the ChangeLog. >>>> >>> >>> I was trying to unify this with glibc. They specify the bug number at >>> the end of the line. >>> >>> We can diverge if it's generally felt to be important, but details like >>> this create needless friction for folk working in both communities. >> >> +1 for "component: Summary [PRnnnnn]" FWIW. >> >> PR bz-component/nnnnn works well for C++. The problem is that so many >> other PRs come under tree-optimization and rtl-optimization, which >> eat up a lot of subject line characters without narrowing things down >> very much. "cselib: ... [PRnnnnn]" is both shorter and more descriptive >> than "PR rtl-optimization/nnnnn - ....", etc. > > Yeah, the cselib version definitely looks preferable to me. > > What if a patch touches more than just the C++ FE, do we want "c,c++:"? c-common: or c-family:? > Though kernel uses > > net: sched: act_ctinfo: fix memory leak I'd read that as a patch that crosses the three separate components. > locking/rwsem: Fix kernel crash when spinning on RWSEM_OWNER_UNKNOWN And this would be a patch that affectst the rwsem sub-component of locking. > > If a patch touches various spots in the optimizers, maybe we can > just go with "tree-opt:" or "rtl:"? > Not sure we want to get that prescriptive. Things are likely to change around anyway. rtl: would suggest it was one of the non-specific rtl-related issues, tree: similarly for a tree-related issue. > Further, I suppose multiple PRs fixed by a single patch would look like: > > c++: Implement DR 666 [PR57, PR12345] Depends on the overall context. In the subject line I think it would be acceptable to reference just one, perhaps two PRs. If there are more, then something like [PRnnn, ...] would probably be an acceptable way of showing that more were referenced later in the body. > > Marek >
diff --git a/htdocs/contribute.html b/htdocs/contribute.html index 042ff069..861f7e5e 100644 --- a/htdocs/contribute.html +++ b/htdocs/contribute.html @@ -249,13 +249,98 @@ that ChangeLog entries may be included as part of the patch and diffs representing new files may be omitted, especially if large, since they can be accessed directly from the repository.)</p> +<h3>E-mail subject lines</h3> + +<p>Your contribution e-mail subject line will become the first line of +the commit message for your patch.</p> + +<p>A high-quality e-mail subject line for a contribution contains the +following elements:</p> + +<ul> + <li>A classifier</li> + <li>Component tags</li> + <li>An optional series identifier</li> + <li>A brief summary</li> + <li>An optional bug number</li> +</ul> + +<h4>Classifier</h4> + +<p>The classifier identifies the type of contribution, for example a +patch, an RFC (request for comments) or a committed patch (where +approval is not necessary. The classifier should be written in upper +case and surrounded with square brackets. This is the only component +of the e-mail subject line that will not appear in the commit itself. +The classifier may optionally contain a version number (v<i>N</i>) and +a series marker (<i>N/M</i>). Examples are:</p> + +<ul> + <li><code>[PATCH]</code> - a single patch</li> + <li><code>[PATCH v2]</code> - the second version of a single patch</li> + <li><code>[PATCH 3/7]</code> - the third patch in a series of seven + patches</li> + <li><code>[RFC]</code> - a point of discussion, may contain a patch</li> + <li><code>[COMMITTED]</code> - a patch that has already been committed.</li> +</ul> + +<h4>Component tags</h4> + +<p>A component tag is a short identifier that identifies the part of +the compiler being modified. This highlights to the relevant +maintainers that the patch may need their attention. Multiple +components may be listed if necessary. Each component tag should be +followed by a colon. For example,</p> + +<ul> + <li><code>libstdc++:</code></li> + <li><code>combine:</code></li> + <li><code>vax: testsuite:</code></li> +</ul> + +<h4>Series identifier</h4> + +<p>The series identifier is optional and is only relevant if a number of +patches are needed in order to effect an overall change. It should be +a <i>short</i> string that identifies the series (it is common to all +patches) and should be followed by a single dash surrounded by white +space.</p> + +<h4>Brief summary</h4> + +<p>The brief summary encapsulates in a few words the intent of the +change. For example: <code>cleanup check_field_decls</code>.</p> + +<h4>Bug number</h4> + +<p>If your patch fixes a bug in the compiler for which there is an +existing PR number the bug number should be stated. Use the +short-form variant (PR<i>nnnnn</i>) without the bugzilla component +identifier.</p> + +<h4>Other messages</h4> + +<p>Some large patch sets benefit from an introductory e-mail that +provides more context for the patch series and describes how the +patches have been broken up to provide for review. The convention is +that such messages should follow the same format as described above, +but the patch number should be set to zero, for example: <code>[PATCH +0/7]</code>. Remember that the introductory message will not be +committed with the patches themselves, so it should not contain any +important information that is not also covered in the individual +patches. If you send a summary e-mail with a series it is a good idea +to send the patches as follow-ups (essentially replies) to your +initial message so that mail software can group the messages +together.</p> + <h3>Pinging patches, Getting patches applied</h3> <p>If you do not receive a response to a patch that you have submitted within two weeks or so, it may be a good idea to chase it by sending a -follow-up email to the same list(s). Patches can occasionally fall through -the cracks. Please be sure to include a brief summary of the patch and the -URL of the entry in the mailing list archive of the original submission.</p> +follow-up e-mail to the same list(s). Patches can occasionally fall +through the cracks. Please be sure to include a brief summary of the +patch and the URL of the entry in the mailing list archive of the +original submission.</p> <p>If you do not have write access and a patch of yours has been approved, but not committed, please advise the approver of that fact. You may want
[updated, following some comments from Gerald, main differences are slight tweaks to the html markup and changing "email" to "e-mail"] This patch proposes some new (additional) rules for email subject lines when contributing to GCC. The goal is to make sure that, as far as possible, the subject for a patch will form a good summary when the message is committed to the repository if applied with 'git am'. Where possible, I've tried to align these rules with those already in use for glibc, so that the differences are minimal and only where necessary. Some things that differ from existing practice (at least by some people) are: - Use '<topic>:' rather than '[<topic>]' - This is more git friendly and works with 'git am'. - Put bug numbers at the end of the line rather than the beginning. - The bug number is useful, but not as useful as the brief summary. Also, use the shortened form, as the topic part is more usefully conveyed in the proper topic field (see above).