On Wed, Jun 04, 2025 at 08:44:55AM +0200, Markus Armbruster wrote: > Alex Bennée <alex.ben...@linaro.org> writes: > > > Markus Armbruster <arm...@redhat.com> writes: > > > >> From: Daniel P. Berrangé <berra...@redhat.com> > >> > >> Currently we have a short paragraph saying that patches must include > >> a Signed-off-by line, and merely link to the kernel documentation. > >> The linked kernel docs have a lot of content beyond the part about > >> sign-off an thus are misleading/distracting to QEMU contributors. > >> > >> This introduces a dedicated 'code-provenance' page in QEMU talking > >> about why we require sign-off, explaining the other tags we commonly > >> use, and what to do in some edge cases. > >> > >> Signed-off-by: Daniel P. Berrangé <berra...@redhat.com> > >> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > >> Signed-off-by: Markus Armbruster <arm...@redhat.com> > >> --- > >> docs/devel/code-provenance.rst | 218 ++++++++++++++++++++++++++++++ > >> docs/devel/index-process.rst | 1 + > >> docs/devel/submitting-a-patch.rst | 18 +-- > >> 3 files changed, 221 insertions(+), 16 deletions(-) > >> create mode 100644 docs/devel/code-provenance.rst > >> > >> diff --git a/docs/devel/code-provenance.rst > >> b/docs/devel/code-provenance.rst > >> new file mode 100644 > >> index 0000000000..4fc12061b5 > >> --- /dev/null > >> +++ b/docs/devel/code-provenance.rst
> >> +Other commit tags > >> +~~~~~~~~~~~~~~~~~ > >> + > >> +While the ``Signed-off-by`` tag is mandatory, there are a number of other > >> tags > >> +that are commonly used during QEMU development: > >> + > >> + * **``Reviewed-by``**: when a QEMU community member reviews a patch on > >> the > >> + mailing list, if they consider the patch acceptable, they should send > >> an > >> + email reply containing a ``Reviewed-by`` tag. Subsystem maintainers who > >> + review a patch should add this even if they are also adding their > >> + ``Signed-off-by`` to the same commit. > >> + > >> + * **``Acked-by``**: when a QEMU subsystem maintainer approves a patch > >> that > >> + touches their subsystem, but intends to allow a different maintainer to > >> + queue it and send a pull request, they would send a mail containing a > >> + ``Acked-by`` tag. Where a patch touches multiple subsystems, > >> ``Acked-by`` > >> + only implies review of the maintainers' own areas of responsibility. > >> If a > >> + maintainer wants to indicate they have done a full review they should > >> use > >> + a ``Reviewed-by`` tag. > >> + > >> + * **``Tested-by``**: when a QEMU community member has functionally > >> tested the > >> + behaviour of the patch in some manner, they should send an email reply > >> + containing a ``Tested-by`` tag. > >> + > >> + * **``Reported-by``**: when a QEMU community member reports a problem > >> via the > >> + mailing list, or some other informal channel that is not the issue > >> tracker, > >> + it is good practice to credit them by including a ``Reported-by`` tag > >> on > >> + any patch fixing the issue. When the problem is reported via the GitLab > >> + issue tracker, however, it is sufficient to just include a link to the > >> + issue. > > > > We don't mention the Link: or Message-Id: tags. > > Yes, but should it go into code-provenance.rst or > submitting-a-patch.rst? I considered those other general tags to be under the scope of submitting-a-patch.rst, as they're not directly related to the legal code provenance. > > You asked for guidance on use of "Message-Id:" in your review of v2. I > understand the practice, and can write guidance, but I wanted to get > this out before my vacation next week, so I left it for later, as > mentioned in the cover letter. > > How do we use "Link:"? What about "Closes:"? > > Here's what the kernel's submitting-patches.rst has to say: > > Describe your changes > --------------------- > > [...] > > If related discussions or any other background information behind the > change > can be found on the web, add 'Link:' tags pointing to it. If the patch is > a > result of some earlier mailing list discussions or something documented > on the > web, point to it. > > When linking to mailing list archives, preferably use the lore.kernel.org > message archiver service. To create the link URL, use the contents of the > ``Message-ID`` header of the message without the surrounding angle > brackets. > For example:: > > Link: > https://lore.kernel.org/30th.anniversary.rep...@klaava.helsinki.fi > > Please check the link to make sure that it is actually working and points > to the relevant message. > > However, try to make your explanation understandable without external > resources. In addition to giving a URL to a mailing list archive or bug, > summarize the relevant points of the discussion that led to the > patch as submitted. > > In case your patch fixes a bug, use the 'Closes:' tag with a URL > referencing > the report in the mailing list archives or a public bug tracker. For > example:: > > Closes: https://example.com/issues/1234 > > Some bug trackers have the ability to close issues automatically when a > commit with such a tag is applied. Some bots monitoring mailing lists can > also track such tags and take certain actions. Private bug trackers and > invalid URLs are forbidden. > > and > > Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes: > ---------------------------------------------------------------------- > > The Reported-by tag gives credit to people who find bugs and report them > and it > hopefully inspires them to help us again in the future. The tag is > intended for > bugs; please do not use it to credit feature requests. The tag should be > followed by a Closes: tag pointing to the report, unless the report is not > available on the web. The Link: tag can be used instead of Closes: if the > patch > fixes a part of the issue(s) being reported. Note, the Reported-by tag is > one > of only three tags you might be able to use without explicit permission > of the > person named (see 'Tagging people requires permission' below for details). > > >> +git commands > >> +^^^^^^^^^^^^ > >> + > >> +When creating, or amending, a commit the ``-s`` flag to ``git commit`` > >> will > >> +append a suitable line matching the configured git author details. > >> + > >> +If preparing patches using the ``git format-patch`` tool, the ``-s`` flag > >> can > >> +be used to append a suitable line in the emails it creates, without > >> modifying > >> +the local commits. Alternatively to modify all the local commits on a > >> branch:: > >> + > >> + git rebase master -x 'git commit --amend --no-edit -s' > >> + > > > > Much as I love Emacs I wonder if this next section is worth it given the > > multiple ways you can solve this (I use yas-snippet expansions for > > example). > > Showing one of them could still be useful for less experienced Emacs > users. We could mention it's just of many ways. Yep, IMHO it is worth guiding users to a simple example that works. If they are advanced users of emacs or other editors wanting to figure out other options they can ignore this guidance. > > > If we do want to mention the editors we should probably also mention b4. > > Can do if somebody contributes a suitable configuration snippet. > > Thanks! > > [...] > With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
