[RFC] [wwwdocs] Rewrite docs on commit message and patch format

Segher Boessenkool segher@kernel.crashing.org
Tue Jun 15 14:24:44 GMT 2021


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


More information about the Gcc-patches mailing list