Message ID | 20250318035605.1221-1-twoerner@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | contributor-guide/submit-changes: encourage patch version changelogs | expand |
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
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
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
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.
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
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 --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.
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(+)