qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] docs: introduce dedicated page about code provenance / s


From: Michael S. Tsirkin
Subject: Re: [PATCH 1/2] docs: introduce dedicated page about code provenance / sign-off
Date: Fri, 24 Nov 2023 06:27:32 -0500

On Fri, Nov 24, 2023 at 12:11:30PM +0100, Philippe Mathieu-Daudé wrote:
> On 23/11/23 18:33, Michael S. Tsirkin wrote:
> > On Thu, Nov 23, 2023 at 05:16:45PM +0000, Daniel P. Berrangé wrote:
> > > On Thu, Nov 23, 2023 at 09:25:13AM -0500, Michael S. Tsirkin wrote:
> > > > On Thu, Nov 23, 2023 at 11:40:25AM +0000, Daniel P. Berrangé wrote:
> > > > > 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 alot of content beyond the part about
> > > > > sign-off an thus is 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é <berrange@redhat.com>
> > > > 
> > > 
> > > > > +  * The non-primary author's contributions were so trivial that
> > > > > +    they can be considered not subject to copyright. In this case
> > > > > +    the secondary authors need not include a ``Signed-off-by``.
> > > > > +
> > > > > +    This case most commonly applies where QEMU reviewers give short
> > > > > +    snippets of code as suggested fixes to a patch. The reviewers
> > > > > +    don't need to have their own ``Signed-off-by`` added unless
> > > > > +    their code suggestion was unusually large.
> > > > 
> > > > It is still a good policy to include attribution, e.g.
> > > > by adding a Suggested-by tag.
> > > 
> > > Will add this tag.
> 
> Thanks!
> 
> > > > > +Other commit tags
> > > > > +~~~~~~~~~~~~~~~~~
> 
> 
> > > > As long as we are here, let's document Fixes: and Cc: ?
> > > 
> > > The submitting-a-patch doc covers more general commit message information.
> > > I think this doc just ought to focus on tags that identify humans involved
> > > in the process.
> > > 
> > > I've never been sure what the point of the 'Cc' tag is, when you actually
> > > want to use the Cc email header ?
> > > 
> > 
> > It records the fact that these people have been copied but did not
> > respond.
> This might be felt aggressive or forcing.
> My understanding of this Cc
> tag in a commit is "now that it is merged, you can't complain". We can
> be absent, sick, on holidays... If I missed a merged patch review I'll
> try to kindly ask on the list if it can be reworked, or suggest a patch
> to fix what I missed.

> Not sure this is really useful to commit that to the repository.

I don't see it as forcing. Sometimes I do a fly-by review of a patch
that caught my eye not in my area. Later people address my comments
and start copying me but I don't have time to re-review.
Recoding the fact that they copied me seems important.

This info might be helpful in git history for other reasons
- helps looking for someone to help review backports
- to guess at code quality - to help understand whether code had all the needed
  people copied


> 
> IMHO the only useful Cc tag is for qemu-stable@nongnu.org, as Kevin
> mentioned.
> 
> If you want to be sure your patch is Cc to a set of developers, you can
> add Cc: lines below the '---' patch separator. My 2 cents eh...
> 
> Regards,
> 
> Phil.


If people feel threatened by CC I don't have a problem to ask people
to put it in a note so it comes after ---.

-- 
MST




reply via email to

[Prev in Thread] Current Thread [Next in Thread]