| Message ID | YMeDFA8ikZL2McG+@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,wwwdocs] Rewrite docs on commit message and patch format | expand |
On 6/14/21 10:25 AM, Jonathan Wakely via Gcc-patches wrote: > I think this is an improvement on the current structure of the docs, > but I'd like to hear what others think. The text looks more detailed and arguably more accurate but also makes it sound more complicated and rigid than necessary. It also doesn't look like the commit hook tries to enforce many of these elements. If it did, quite a number of commits would fail. So I'm not sure about the value of documenting expectations that only few commits would meet. E.g., including the Component tag, or putting PRnnnnn at the end of the Subject line with no space (why ask for no space and not, for example PR #nnnnn?) In fact, unless we mean it (and are willing to enforce it) I think it would be best to either leave it out completely, or make it clear that it's not required. Martin > > We don't currently say document anything about commit format for the > wwwdocs repo. Should the "wwwdocs" be a classifier (as in this email) > or a component tag? > >
On 14/06/21 17:25 +0100, Jonathan Wakely wrote: >I think this is an improvement on the current structure of the docs, >but I'd like to hear what others think. Slightly improved version, moving the "this highlights to the relevant maintainers that the patch might need their attention" back to the patch email docs, and adding rationale for good one-line subjects.
On Tue, 15 Jun 2021 at 01:12, Martin Sebor wrote: > > On 6/14/21 10:25 AM, Jonathan Wakely via Gcc-patches wrote: > > I think this is an improvement on the current structure of the docs, > > but I'd like to hear what others think. > > The text looks more detailed and arguably more accurate but also > makes it sound more complicated and rigid than necessary. It > also doesn't look like the commit hook tries to enforce many of > these elements. If it did, quite a number of commits would fail. > > So I'm not sure about the value of documenting expectations that > only few commits would meet. We don't have to be too strict, but encouraging good practice still makes the commit logs more useful. Even if only 50% of commit follow it, that still seems to make the logs easier to skim than if there is no consistency at all. > E.g., including the Component tag, > or putting PRnnnnn at the end of the Subject line with no space > (why ask for no space and not, for example PR #nnnnn?) In fact, That was always there, I just moved it from one page to another. I have traditionally used a space before the bug number, so I'm fine with that format, but I don't really think it makes the docs better to list too many variations. And I am not trying to make big changes to the policy with this patch, just reorganize the existing docs to reflect the modern workflow (i.e. Git commits with automatically generated ChangeLog files, rather than everything being about the ChangeLog). > unless we mean it (and are willing to enforce it) I think it > would be best to either leave it out completely, or make it clear > that it's not required. If we don't enforce it, then it's not required. Commits that don't do it will still get in. I think suggesting a single format (but allowing variations on it) is **much** better than not saying anything at all. For new contributors it's helpful to say "this is what we want" so they have a guideline to follow. My revised patch sent a few minutes ago adds: <p>A major benefit of a good, descriptive subject line is that it makes the output of <code>git log --oneline</code> more useful. Including the component tag and bug number(s) helps with that, but isn't compulsory if the subject is already clear and has enough context.</p> Does that make you happier?
On Mon, Jun 14, 2021 at 05:25:56PM +0100, Jonathan Wakely via Gcc-patches wrote: > We don't currently say document anything about commit format for the > wwwdocs repo. Should the "wwwdocs" be a classifier (as in this email) > or a component tag? I use proper components for wwwdocs as well, and when I send the patch to gcc-patches@ I replace the [PATCH] by [wwwdocs]. It shouls not end up in the repository after all, but we do need it in the mail. > +<h4 id="Subject">Subject line</h4> > + > +<p>The first line of the commit message contains a brief summary that > +encapsulates the intent of the change. > +For example: <code>cleanup check_field_decls</code>. > +Although, very short, the summary should be distinct so that it will > +not be confused with other patches.</p> > + > +Additionally, the subject should begin with a component tag, followed by > +an optional series identifier. When related to one or more bugs, > +it should end with the bug numbers. > + > +<p>Ideally, the entire subject line should not exceed 75 characters.</p> This is very wrong, and the cause of all other imperfections in this proposal: the ideal subject line length is only 50 chars, so that all important info in it shows up in a mail reader, and in gitk, tig, git log --oneline, whatnot. This has been a settled debate since forever and a day, don't try to redo it :-/ > +<h4 id="Component">Component tags</h4> If done properly you can put more info in a subject line by starting it with the area the patch is about. If done improperly you can blow the 50 chars length limit here already. > +<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 > +commits in the series) and should be followed by a single dash surrounded > +by white space.</p> A "series identifier" does not belong in commits, only in the emails, and you should use a tool (like git send-email) that handles this automatically. And the exact format is [PATCH m/n]. Sometimes people add a "v2" or similar in there -- stuff in square brackets is deleted when the patch is committed, so that is fine. Most other decoorations are useless, would only matter if you send more than a few 100+-patch series daily (which I would recommend against for other reasons :-) ) > +<h4>Bug number</h4> > + > +<p>If your patch is related to a bug in the compiler for which there is an > +existing PR number, the bug number should be stated. Use the The bug number *can* be stated. > +short-form variant <code>[PR<i>nnnnn</i>]</code> without the bugzilla > +component identifier and with no space between 'PR' and the number. > +The body of the commit message should still contain the full form > +(<code>PR <component>/<i>nnnnn</i></code>) so that Bugzilla > +will correctly notice the commit. Not the body anywhere, it should be in the changelog. > +If your patch relates to two bugs, then write > +<code>[PR<i>nnnnn</i>, PR<i>mmmmm</i>]</code>. This is too long usually. The most important thing is this should be at the *end* of the subject, so that more important info is readable. It can be nice if it is in the subject of course, makes it a bit easier to search for, but it is mostly irrelevant when scanning the commit log, or reading emails even. I know what bug PR323 is, but we have six-digit numbers now, you will recognise only the few you spent most time with recently. - - - IMNSHO this all should emphasise *why* these things are recommended, and don't pretend these are "rules" at all. They are not. The important thing is to make it easy for *others* to work with your patches. Adding frivolities does not help; concise, well-phrased, terse *does* help. Segher
On Tue, Jun 15, 2021 at 10:30:05AM +0100, Jonathan Wakely via Gcc-patches wrote: > Slightly improved version, moving the "this highlights to the relevant > maintainers that the patch might need their attention" back to the > patch email docs, and adding rationale for good one-line subjects. Ah, anmd don't send patches as replies, certainly not without changing the email subject! I did not notice there was an updated patch downthread when I sent my reply. Segher
On Mon, Jun 14, 2021 at 06:12:24PM -0600, Martin Sebor via Gcc-patches wrote: > The text looks more detailed and arguably more accurate but also > makes it sound more complicated and rigid than necessary. +1 > (why ask for no space and not, for example PR #nnnnn?) Because it is shorter. If your lines are too long (>50 chars) the info will not show up in lists; if that is okay that info does not belong in the subject at all. The subject is there to identify what the patch is about, what the patch *is*, both for people who have seen it before and for people who have not. We need guidelines that encourage people to write good subjects with that single goal in mind, and all other guidelines (or "rules") are counterproductive. Segher
On Tue, Jun 15, 2021 at 10:39:58AM +0100, Jonathan Wakely via Gcc-patches wrote: > I think suggesting a single format (but allowing variations on it) is > **much** better than not saying anything at all. For new contributors > it's helpful to say "this is what we want" so they have a guideline to > follow. Most people are smart enough to look at what others do and think about why they do things that way, I hope. And then think what is best for themselves. Maybe I am too optimistic :-) > <p>A major benefit of a good, descriptive subject line is that it makes > the output of <code>git log --oneline</code> more useful. Including the > component tag and bug number(s) helps with that, but isn't compulsory > if the subject is already clear and has enough context.</p> The tags and the PR # are the least important things in the patch subject. If you can fit them in (without blowing the 50 char limit) then sure; and as I said before, well-chosen headings can be shorter than free-form text expressing the same. Segher
On Tue, 15 Jun 2021 at 14:03, Segher Boessenkool wrote: > > On Mon, Jun 14, 2021 at 05:25:56PM +0100, Jonathan Wakely via Gcc-patches wrote: > > We don't currently say document anything about commit format for the > > wwwdocs repo. Should the "wwwdocs" be a classifier (as in this email) > > or a component tag? > > I use proper components for wwwdocs as well, and when I send the patch > to gcc-patches@ I replace the [PATCH] by [wwwdocs]. It shouls not end > up in the repository after all, but we do need it in the mail. > > > +<h4 id="Subject">Subject line</h4> > > + > > +<p>The first line of the commit message contains a brief summary that > > +encapsulates the intent of the change. > > +For example: <code>cleanup check_field_decls</code>. > > +Although, very short, the summary should be distinct so that it will > > +not be confused with other patches.</p> > > + > > +Additionally, the subject should begin with a component tag, followed by > > +an optional series identifier. When related to one or more bugs, > > +it should end with the bug numbers. > > + > > +<p>Ideally, the entire subject line should not exceed 75 characters.</p> > > This is very wrong, and the cause of all other imperfections in this > proposal: the ideal subject line length is only 50 chars, so that all > important info in it shows up in a mail reader, and in gitk, tig, > git log --oneline, whatnot. This has been a settled debate since > forever and a day, don't try to redo it :-/ The existing docs say 75 chars: https://gcc.gnu.org/contribute.html#patches The frustrating thing about this proposal is that all I'm really trying to do is restructure the existing policies to be documented in more sensible places, and everybody is picking holes in the existing policy as though I'm introducing it. > > > +<h4 id="Component">Component tags</h4> > > If done properly you can put more info in a subject line by starting it > with the area the patch is about. If done improperly you can blow the > 50 chars length limit here already. > > > +<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 > > +commits in the series) and should be followed by a single dash surrounded > > +by white space.</p> > > A "series identifier" does not belong in commits, only in the emails, > and you should use a tool (like git send-email) that handles this > automatically. And the exact format is [PATCH m/n]. Sometimes people > add a "v2" or similar in there -- stuff in square brackets is deleted > when the patch is committed, so that is fine. I didn't invent this, it's the existing policy: https://gcc.gnu.org/contribute.html#patches The [PATCH v2] or [PATCH m/n] is part of the "classifier", which is separate from the "series identifier" you're objecting to. The m/n part is called a "series marker" within the "classifier". > > > +<h4>Bug number</h4> > > + > > +<p>If your patch is related to a bug in the compiler for which there is an > > +existing PR number, the bug number should be stated. Use the > > The bug number *can* be stated. Again, you're objecting to the current text: https://gcc.gnu.org/contribute.html#patches > > > +short-form variant <code>[PR<i>nnnnn</i>]</code> without the bugzilla > > +component identifier and with no space between 'PR' and the number. > > +The body of the commit message should still contain the full form > > +(<code>PR <component>/<i>nnnnn</i></code>) so that Bugzilla > > +will correctly notice the commit. > > Not the body anywhere, it should be in the changelog. That's the current text: https://gcc.gnu.org/contribute.html#patches > > +If your patch relates to two bugs, then write > > +<code>[PR<i>nnnnn</i>, PR<i>mmmmm</i>]</code>. > > This is too long usually. That's the current text: https://gcc.gnu.org/contribute.html#patches > The most important thing is this should be at the *end* of the subject, > so that more important info is readable. It can be nice if it is in the > subject of course, makes it a bit easier to search for, but it is mostly > irrelevant when scanning the commit log, or reading emails even. I know > what bug PR323 is, but we have six-digit numbers now, you will recognise > only the few you spent most time with recently. > > - - - > > IMNSHO this all should emphasise *why* these things are recommended, and > don't pretend these are "rules" at all. They are not. The important > thing is to make it easy for *others* to work with your patches. Adding > frivolities does not help; concise, well-phrased, terse *does* help. If I was trying to improve the current policy, I would agree. I'm not. I'm just trying to move the existing text around, without significant changes to the content of that text. I'm all for improving the policies, but that's not what I'm trying to do here. I would like to move things to a more sensible place **first** and then we can improve them.
On Tue, Jun 15, 2021 at 02:12:31PM +0100, Jonathan Wakely wrote: > On Tue, 15 Jun 2021 at 14:03, Segher Boessenkool wrote: > > > > On Mon, Jun 14, 2021 at 05:25:56PM +0100, Jonathan Wakely via Gcc-patches wrote: > > > We don't currently say document anything about commit format for the > > > wwwdocs repo. Should the "wwwdocs" be a classifier (as in this email) > > > or a component tag? > > > > I use proper components for wwwdocs as well, and when I send the patch > > to gcc-patches@ I replace the [PATCH] by [wwwdocs]. It shouls not end > > up in the repository after all, but we do need it in the mail. > > > > > +<h4 id="Subject">Subject line</h4> > > > + > > > +<p>The first line of the commit message contains a brief summary that > > > +encapsulates the intent of the change. > > > +For example: <code>cleanup check_field_decls</code>. > > > +Although, very short, the summary should be distinct so that it will > > > +not be confused with other patches.</p> > > > + > > > +Additionally, the subject should begin with a component tag, followed by > > > +an optional series identifier. When related to one or more bugs, > > > +it should end with the bug numbers. > > > + > > > +<p>Ideally, the entire subject line should not exceed 75 characters.</p> > > > > This is very wrong, and the cause of all other imperfections in this > > proposal: the ideal subject line length is only 50 chars, so that all > > important info in it shows up in a mail reader, and in gitk, tig, > > git log --oneline, whatnot. This has been a settled debate since > > forever and a day, don't try to redo it :-/ > > The existing docs say 75 chars: > https://gcc.gnu.org/contribute.html#patches Yes, and that is the most important thing to fix here. > The frustrating thing about this proposal is that all I'm really > trying to do is restructure the existing policies to be documented in > more sensible places, and everybody is picking holes in the existing > policy as though I'm introducing it. You are opening up old wounds ;-) Anyway, the hugely important thing that *should* be improved in the documentation is it should say *why* we recommend to do certain things in some certain way. Which your edit does not make better at all. But it is the *only* thing that matters! More people will follow your recommendations if the goal is clear (and your recommendations make sense for the goal of course :-) ) > > A "series identifier" does not belong in commits, only in the emails, > > and you should use a tool (like git send-email) that handles this > > automatically. And the exact format is [PATCH m/n]. Sometimes people > > add a "v2" or similar in there -- stuff in square brackets is deleted > > when the patch is committed, so that is fine. > > I didn't invent this, it's the existing policy: > > https://gcc.gnu.org/contribute.html#patches If that is the current policy, I have been violating it left and right, and I will coontinue doing that. > The [PATCH v2] or [PATCH m/n] is part of the "classifier", which is > separate from the "series identifier" you're objecting to. The m/n > part is called a "series marker" within the "classifier". I have no idea why we need confusing new terms for basic things we (that is, the bigger community, everyone using Git) have used since forever. > > > +<h4>Bug number</h4> > > > + > > > +<p>If your patch is related to a bug in the compiler for which there is an > > > +existing PR number, the bug number should be stated. Use the > > > > The bug number *can* be stated. > > Again, you're objecting to the current text: > https://gcc.gnu.org/contribute.html#patches Not at all, there are + in front, this is proposed new text. > > IMNSHO this all should emphasise *why* these things are recommended, and > > don't pretend these are "rules" at all. They are not. The important > > thing is to make it easy for *others* to work with your patches. Adding > > frivolities does not help; concise, well-phrased, terse *does* help. ^^^ Let me repeat that, it is all that matters: IMNSHO this all should emphasise *why* these things are recommended, and don't pretend these are "rules" at all. They are not. The important thing is to make it easy for *others* to work with your patches. Adding frivolities does not help; concise, well-phrased, terse *does* help. > If I was trying to improve the current policy, I would agree. I'm not. > I'm just trying to move the existing text around, without significant > changes to the content of that text. There lies your misunderstanding then. This is not policy at all. These are guidelines, and we need to explain the reasoning behind them to get people to follow the spirit (if not the letter) of them. > I would like to move things to a more sensible place > **first** and then we can improve them. You have my support in that, in that case. Segher
On Tue, 15 Jun 2021 at 15:25, Segher Boessenkool wrote: > > Again, you're objecting to the current text: > > https://gcc.gnu.org/contribute.html#patches > > Not at all, there are + in front, this is proposed new text. Moved from one file to another. > > > > IMNSHO this all should emphasise *why* these things are recommended, and > > > don't pretend these are "rules" at all. They are not. The important > > > thing is to make it easy for *others* to work with your patches. Adding > > > frivolities does not help; concise, well-phrased, terse *does* help. > > ^^^ Let me repeat that, it is all that matters: > > IMNSHO this all should emphasise *why* these things are recommended, and > don't pretend these are "rules" at all. They are not. The important > thing is to make it easy for *others* to work with your patches. Adding > frivolities does not help; concise, well-phrased, terse *does* help. Patches welcome, I'm not going to work on this change.
On Tue, Jun 15, 2021 at 09:24:44AM -0500, Segher Boessenkool wrote: > IMNSHO this all should emphasise *why* these things are recommended, and > don't pretend these are "rules" at all. They are not. The important Many of them actually are rules. Everything that is verified by the pre-commit hook and commits rejected if the hook reports some rule not being fulfilled. Jakub
On Tue, Jun 15, 2021 at 05:04:51PM +0200, Jakub Jelinek wrote: > On Tue, Jun 15, 2021 at 09:24:44AM -0500, Segher Boessenkool wrote: > > IMNSHO this all should emphasise *why* these things are recommended, and > > don't pretend these are "rules" at all. They are not. The important > > Many of them actually are rules. > Everything that is verified by the pre-commit hook and commits rejected if > the hook reports some rule not being fulfilled. Many such things were not rules before those scripts existed fwiw. Segher
On 6/15/21 3:39 AM, Jonathan Wakely wrote: > On Tue, 15 Jun 2021 at 01:12, Martin Sebor wrote: >> >> On 6/14/21 10:25 AM, Jonathan Wakely via Gcc-patches wrote: >>> I think this is an improvement on the current structure of the docs, >>> but I'd like to hear what others think. >> >> The text looks more detailed and arguably more accurate but also >> makes it sound more complicated and rigid than necessary. It >> also doesn't look like the commit hook tries to enforce many of >> these elements. If it did, quite a number of commits would fail. >> >> So I'm not sure about the value of documenting expectations that >> only few commits would meet. > > We don't have to be too strict, but encouraging good practice still > makes the commit logs more useful. Even if only 50% of commit follow > it, that still seems to make the logs easier to skim than if there is > no consistency at all. > >> E.g., including the Component tag, >> or putting PRnnnnn at the end of the Subject line with no space >> (why ask for no space and not, for example PR #nnnnn?) In fact, > > That was always there, I just moved it from one page to another. I > have traditionally used a space before the bug number, so I'm fine > with that format, but I don't really think it makes the docs better to > list too many variations. And I am not trying to make big changes to > the policy with this patch, just reorganize the existing docs to > reflect the modern workflow (i.e. Git commits with automatically > generated ChangeLog files, rather than everything being about the > ChangeLog). > >> unless we mean it (and are willing to enforce it) I think it >> would be best to either leave it out completely, or make it clear >> that it's not required. > > If we don't enforce it, then it's not required. Commits that don't do > it will still get in. > > I think suggesting a single format (but allowing variations on it) is > **much** better than not saying anything at all. For new contributors > it's helpful to say "this is what we want" so they have a guideline to > follow. > > My revised patch sent a few minutes ago adds: > > <p>A major benefit of a good, descriptive subject line is that it makes > the output of <code>git log --oneline</code> more useful. Including the > component tag and bug number(s) helps with that, but isn't compulsory > if the subject is already clear and has enough context.</p> > > Does that make you happier? I expect most users to read this text either before they commit their first GCC change or after the commit hook rejects it. At that point, what they'll want to find out as quickly and effortlessly as possible, is what they need to do to get the patch committed without an error. Reading several paragraphs of best practices isn't likely to be the most expedient way to figure that out. What it will be is frustrating, especially if the hard requirements are sprinkled among but not obviously distinguished from the best practices, and if following some but not others doesn't resolve the error. To minimize the frustration I'd suggest to identify the hard requirements and add links showing examples of representative commits that follow the best practices. Then the rest of the text can go over all the details of classifiers and component tags, etc., with more links to example commits. As an aside, adopting the same conventions across all toolchain projects (Binutils/GDB, GCC, and Glibc) would simplify things for contributors to all of them. Martin PS To the point of pulling up git log and using the last commit as a template: that works but unless the last commit follows the best practices it doesn't accomplish what you want.
diff --git a/htdocs/codingconventions.html b/htdocs/codingconventions.html index 21cc95de..6c574a28 100644 --- a/htdocs/codingconventions.html +++ b/htdocs/codingconventions.html @@ -114,10 +114,12 @@ maintained and kept up to date. In particular:</p> <p> ChangeLog entries are part of git commit messages and are automatically put -into a corresponding ChangeLog file. A ChangeLog template can be easily generated -with <code>./contrib/mklog.py</code> script. GCC offers a checking script that -verifies a proper ChangeLog formatting (see <code>git gcc-verify</code> git alias). -for a particular git commit. The checking script covers most commonly used ChangeLog +into a corresponding <code>ChangeLog</code> file. +A ChangeLog template can be easily generated by the +<code>./contrib/mklog.py</code> script. GCC offers a checking script that +verifies proper ChangeLog formatting for a particular git commit +(see the <code>git gcc-verify</code> git alias). +The checking script covers most commonly used ChangeLog formats and the following paragraphs explain what it supports. </p> @@ -129,10 +131,12 @@ in comments rather than the ChangeLog, though a single line overall description of the changes may be useful above the ChangeLog entry for a large batch of changes.</p> -<h3>Components</h3> +<h3>Components of a git commit message</h3> <ul> + <li><code>subject</code> - a brief description of the commit</li> <li><code>git_description</code> - a leading text with git commit description</li> + <li><code>sign_off</code> - <a href="https://gcc.gnu.org/dco.html">DCO sign-off</a></li> <li><code>committer_timestamp</code> - line with timestamp and an author name and email (2 spaces before and after name) <br> example: <code>2020-04-23␣␣Martin Liska␣␣<mliska@suse.cz></code></li> <li><code>additional_author</code> - line with additional commit author name and email (starting with a tabular and 4 spaces) <br> @@ -153,7 +157,9 @@ a large batch of changes.</p> <h3>Format rules</h3> <ul> - <li><code>git_description</code> - optional; ends right before one of the other compoments is found</li> + <li><code>subject</code> - a single line in the format described below</li> + <li><code>git_description</code> - optional; ends right before one of the other components is found</li> + <li><code>sign_off</code> - optional; see <a href="https://gcc.gnu.org/dco.html">DCO</a> policy</li> <li><code>committer_timestamp</code> - optional; when found before a <code>changelog_file</code>, then it is added to each changelog entry</li> <li><code>additional_author</code> - optional</li> @@ -166,6 +172,65 @@ a large batch of changes.</p> <li><code>co_authored_by</code> - optional, can contain more than one</li> </ul> +<h4 id="Subject">Subject line</h4> + +<p>The first line of the commit message contains a brief summary that +encapsulates the intent of the change. +For example: <code>cleanup check_field_decls</code>. +Although, very short, the summary should be distinct so that it will +not be confused with other patches.</p> + +Additionally, the subject should begin with a component tag, followed by +an optional series identifier. When related to one or more bugs, +it should end with the bug numbers. + +<p>Ideally, the entire subject line should not exceed 75 characters.</p> + +<h4 id="Component">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> + +<p>Some large components may be subdivided into sub-components. If +the subcomponent name is not disctinct in its own right, you can use the +form <i>component/sub-component:</i>.</p> + +<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 +commits in the series) and should be followed by a single dash surrounded +by white space.</p> + +<h4>Bug number</h4> + +<p>If your patch is related to a bug in the compiler for which there is an +existing PR number, the bug number should be stated. Use the +short-form variant <code>[PR<i>nnnnn</i>]</code> without the bugzilla +component identifier and with no space between 'PR' and the number. +The body of the commit message should still contain the full form +(<code>PR <component>/<i>nnnnn</i></code>) so that Bugzilla +will correctly notice the commit. +If your patch relates to two bugs, then write +<code>[PR<i>nnnnn</i>, PR<i>mmmmm</i>]</code>. +For more than two bugs just cite the most relevant one in the summary +and use an ellipsis after the first one; list all the +related PRs in the body of the commit message in the full form.</p> + +<p>It is not necessary to cite bugs that are closed as duplicates of +another unless there is something specific to a duplicate report that +is not covered by its parent bug.</p> + <h3>Documented behaviour</h3> <ul> @@ -182,7 +247,9 @@ a large batch of changes.</p> <h3>Example patch</h3> -<pre><code>This patch adds a second movk pattern that models the instruction +<pre><code>aarch64: Add an extra sbfiz pattern [PR87763] + +This patch adds a second movk pattern that models the instruction as a "normal" and/ior operation rather than an insertion. It fixes the third insv_1.c failure in PR87763, which was a regression from GCC 8. @@ -205,7 +272,11 @@ Co-Authored-By: Jack Bar <jack@example.com> <h3>Tokenized patch</h3> <pre> -<code>$git_description +<code>$subject + +$git_description + +$sign_off $committer_timestamp diff --git a/htdocs/contribute.html b/htdocs/contribute.html index c0cce359..b879b2d5 100644 --- a/htdocs/contribute.html +++ b/htdocs/contribute.html @@ -243,29 +243,17 @@ 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> - -<p>The entire line (excluding the classifier) should not exceed 75 -characters.</p> +the commit message for your patch, so should follow the same +<a href="codingconventions.html#Subject">format</a>, +prefixed by a classifier.</p> <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. +<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 surrounded with square brackets, +and is often written all upper case. 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> @@ -275,61 +263,9 @@ a series marker (<i>N/M</i>). Examples are:</p> <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> + <li><code>[committed]</code> - a patch that has already been committed.</li> </ul> -<p>Some large components may be subdivided into sub-components. If -the subcomponent name is not disctinct in its own right, you can use the -form <i>component/sub-component:</i>.</p> - -<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>A Very 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>. -Although, very short, the summary should be distinct so that it will -not be confused with other patches.</p> - -<h4>Bug number</h4> - -<p>If your patch relates a bug in the compiler for which there is an -existing PR number the bug number should be stated. Use the -short-form variant <code>[PR<i>nnnnn</i>]</code> without the bugzilla -component identifier and with no space between 'PR' and the number. -The body of the commit message should still contain the full form -(<code>PR <component>/<i>nnnnn</i></code>) within the body of -the commit message so that Bugzilla will correctly notice the -commit. If your patch relates to two bugs, then write -<code>[PR<i>nnnnn</i>, PR<i>mmmmm</i>]</code>. For multiple -bugs, just cite the most relevant one in the summary and use an -elipsis instead of the second, or subsequent PR numbers; list all the -related PRs in the body of the commit message in the normal way.</p> - -<p>It is not necessary to cite bugs that are closed as duplicates of -another unless there is something specific to that report that -is not covered by the parent bug.</p> - <h4>Other messages</h4> <p>Some large patch sets benefit from an introductory e-mail that @@ -362,7 +298,7 @@ original thread indicating that a new version has been submitted.</p> <p>Here are some complete examples, based on real commits to GCC.</p> <ul> - <li><code>[COMMITTED] libstdc++: Fix freestanding build [PR92376]</code></li> + <li><code>[committed] libstdc++: Fix freestanding build [PR92376]</code></li> <li><code>[PATCH 1/6] analyzer: Fix tests for UNKNOWN_LOCATION</code></li> <li><code>[RFC] doc: Note that some warnings depend on optimizations [PR92757]</code></li> <li><code>[COMMITTED] c++: coroutines - Initial implementation</code></li>
I think this is an improvement on the current structure of the docs, but I'd like to hear what others think. We don't currently say document anything about commit format for the wwwdocs repo. Should the "wwwdocs" be a classifier (as in this email) or a component tag? commit 297dfc7049e5885de9ada2bf65235a13a74ff23e Author: Jonathan Wakely <jwakely@redhat.com> Date: Mon Jun 14 17:16:48 2021 +0100 Rewrite docs on commit message and patch format Move text about subject lines from contribute.html to codingconventions.html and then refer to it. Document DCO sign-off in commit message.