diff mbox series

contributor-guide/submit-changes: encourage patch version changelogs

Message ID 20250318035605.1221-1-twoerner@gmail.com
State Superseded
Headers show
Series contributor-guide/submit-changes: encourage patch version changelogs | expand

Commit Message

Trevor Woerner March 18, 2025, 3:56 a.m. UTC
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(+)

Comments

Antonin Godard March 18, 2025, 8:08 a.m. UTC | #1
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)

> +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?

> +   ---
> +   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

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.

> +   ---
> +   <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
Quentin Schulz March 18, 2025, 10:56 a.m. UTC | #2
Hi Trevor,

On 3/18/25 4:56 AM, 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
> +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::
> +

If you're looking at this from a maintainer perspective, maybe consider 
using b4.

For reviewing stuff, I sometimes do

b4 diff -v 1 2 -- <lore.kernel.org URL to v2>

and that helps.

This is orthogonal to this patch though but just wanted to mention it :)

Cheers,
Quentin
Trevor Woerner March 21, 2025, 4:51 p.m. UTC | #3
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
Trevor Woerner March 21, 2025, 4:54 p.m. UTC | #4
On Tue 2025-03-18 @ 11:56:47 AM, Quentin Schulz wrote:
> Hi Trevor,
> 
> On 3/18/25 4:56 AM, 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
> > +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::
> > +
> 
> If you're looking at this from a maintainer perspective, maybe consider
> using b4.
> 
> For reviewing stuff, I sometimes do
> 
> b4 diff -v 1 2 -- <lore.kernel.org URL to v2>
> 
> and that helps.
> 
> This is orthogonal to this patch though but just wanted to mention it :)

Thanks for the reminder! I've been meaning to force myself to start using b4.
We're pretty lucky that our repositories are hosted on lore.kernel.org. We
even have our own lore repository at work internally, so b4 would be useful
there too.
Antonin Godard March 24, 2025, 8:03 a.m. UTC | #5
Hi Trevor,

On Fri Mar 21, 2025 at 5:51 PM CET, Trevor Woerner wrote:
[...]
>> > +   ---
>> > +   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".

Oh ok, I didn't understand what you wanted to show when reading the example. I
think for the sake of the example we could be a little more verbose, and write
something like: "added patch '<title of the new patch>' to the series". Again,
it's just to give clear examples to users of what this changelog could contain.
What do you think?

>> 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.

Actually, b4 is doing this for me automatically. :)

Thanks,
Antonin
Antonin Godard April 10, 2025, 12:22 p.m. UTC | #6
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.
[...]

Do you plan on sending a new version of this patch? :)
(Did I not receive it?)

Antonin
diff mbox series

Patch

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
+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>
+   ---
+   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
+   ---
+   <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.