Re: [docs] [PATCH] contributor-guide/submit-changes: encourage patch version changelogs

[Trevor Woerner <twoerner@gmail.com>]

On Tue 2025-03-18 @ 09:08:12 AM, Antonin Godard wrote:
> Hi Trevor,
> 
> On Tue Mar 18, 2025 at 4:56 AM CET, Trevor Woerner via lists.yoctoproject.org wrote:
> > Add a section after the 'git format-patch' information encouraging developers
> > to add patch version changelogs to their patch updates.
> >
> > Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> > ---
> >  .../contributor-guide/submit-changes.rst      | 32 +++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/documentation/contributor-guide/submit-changes.rst b/documentation/contributor-guide/submit-changes.rst
> > index 0675aac984cf..5ade6ba5cdef 100644
> > --- a/documentation/contributor-guide/submit-changes.rst
> > +++ b/documentation/contributor-guide/submit-changes.rst
> > @@ -776,6 +776,38 @@ argument to ``git format-patch`` with a version number::
> >  
> >     git format-patch -v2 <ref-branch>
> >  
> > +
> > +After generating updated patches (v2, v3, and so on) via ``git
> 
> s/via/with/? (to use simpler phrasings)

Yes, I too ran vale against this text. I saw that suggestion and chose to
ignore it ;-)

> > +format-patch``, ideally developers will add a patch version changelog
> > +to each patch that describes what has changed between each revision of
> > +the patch. Add patch version changelogs after the ``---`` marker in the
> > +patch, indicating that this information is part of this patch, but is not
> > +suitable for inclusion in the commit message (i.e. the git history) itself.
> > +Providing a patch version changelog makes it easier for maintainers and
> > +reviewers to succinctly understand what changed in all versions of the
> > +patch, without having to consult alternate sources of information, such as
> > +searching through messages on a mailing list. For example::
> > +
> > +   <patch title>
> > +
> > +   <commit message>
> > +
> > +   <SoB/etc lines>
> 
> Maybe write "<Signed-off-by/other trailers>" so there is no doubt about the
> meaning of SoB?

Okay.

> > +   ---
> > +   changes in v4:
> > +   - provide a clearer commit message
> > +   - fix spelling mistakes
> > +
> > +   changes in v3:
> > +   - replace func() to use other_func() instead
> > +
> > +   changes in v2:
> > +   - added
> 
> "added return value check for func"?
> to give a more concrete example

It's not uncommon for the v1 series to have N patches, and the v2 series to
have >N patches; in which case one or more patches were added in v2. This
isn't a really bad commit log where the author forgets to say what they added,
it's a message saying "this patch was added in v2".

> I think it would also be nice to also instrict to give the link to the previous
> patch but that might go a bit too far if done manually each time.

That's not something I've seen often, but it could be community-specific.

> > +   ---
> > +   <diffstat output>
> > +
> > +   <unified diff>
> > +
> >  Lastly please ensure that you also test your revised changes. In particular
> >  please don't just edit the patch file written out by ``git format-patch`` and
> >  resend it.
> 
> Otherwise this looks good to me, these are simply suggestions.
> 
> Thank you,
> Antonin
> 
> -- 
> Antonin Godard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com