Message-ID: <20170224221318.GB30506@wisp>
Date:   Fri, 24 Feb 2017 14:13:18 -0800
From:   Darren Hart <dvhart@...radead.org>
To:     Michał Kępień <kernel@...pniu.pl>
Cc:     Jonathan Woithe <jwoithe@...t42.net>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Andy Shevchenko <andy@...radead.org>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 00/10] fujitsu-laptop: renames and cleanups

On Fri, Feb 17, 2017 at 08:14:51AM +0100, Michał Kępień wrote:
> > On Fri, Feb 17, 2017 at 01:38:04PM +1030, Jonathan Woithe wrote:
> > > On Thu, Feb 16, 2017 at 06:57:08PM -0800, Darren Hart wrote:
> > > > On Fri, Feb 10, 2017 at 02:42:00AM +0200, Andy Shevchenko wrote:
> > > > > On Fri, Feb 10, 2017 at 2:16 AM, Jonathan Woithe <jwoithe@...t42.net> wrote:
> > > > > > On Wed, Feb 08, 2017 at 02:46:23PM +0100, Micha?? K??pie?? wrote:
> > > > > 
> > > > > > In summary, I see no issues with this patch series which provides a much
> > > > > > needed clean up of the code and naming conventions within the fujitsu-laptop
> > > > > > driver.  I'm happy for this series (patches 1-10/10) to be applied.
> > > > > >
> > > > > > Signed-off-by: Jonathan Woithe <jwoithe@...t42.net>
> > > > > 
> > > > > I have noticed people start using SoB for the code they are
> > > > > maintaining w/o sending any pull requests.
> > > > > It is okay, but there is, as Wolfram pointed, a downside for patchwork
> > > > > users. Patchwork is tracking tags (A/R/T) which by a glance allows to
> > > > > see what patches are acked/reviewed/tested.
> > > > 
> > > > Signed-off-by tracks the path the code takes from author to mainline. If you are
> > > > not the author or committing it to a tree followed by a pull-request, the
> > > > correct tag is "Reviewed-by".
> > > 
> > > Yes, of course - I clearly had a brain fade back there.  Having said that, 
> > > in the past I've used "Acked-by" intead of "Reviewed-by".
> > 
> > :-)
> > 
> > > Do you want me to continue to use Acked-by, or should I switch to
> > > Reviewed-by?
> > 
> > These tags do have different meanings, and have come up at Kernel Summit the
> > last couple of years. My interpretation of those discussions is:
> > 
> > Acked-by: I have no objection to this patch, but I didn't really give it a
> > thorough review. I trust your judgement. e.g. minor change to your driver to
> > support a subsystem API change. These are of very little value.
> > 
> > Reviewed-by: I have carefully reviewed this patch and would like it to be
> > applied. This should usually come with some sort of commentary describing the
> > level of review or an area you focused on. This is what we would like to see
> > from all of our driver maintainers. These are high value.
> > 
> > Linus *really* dislikes one line acked by's, and only *slightly* more so than
> > one line reviewed by's. :-)
> 
> This is really useful information and I think it does not deserve to be
> forgotten in a mailing list archive.  If this is indeed the status quo,
> Documentation/process/submitting-patches.rst could use some love.  Here
> is what it currently says:
> 
> > Acked-by: is often used by the maintainer of the affected code when that
> > maintainer neither contributed to nor forwarded the patch.
> 
> My short experience with the x86 platform driver subsystem is consistent
> with that.  The informal rule I inferred from mailing list discussions
> is that Acked-by: means the maintainer has reviewed the patch and sees
> no objections to it being applied.
> 
> Granted, Documentation/process/submitting-patches.rst also states that:
> 
> > Acked-by: does not necessarily indicate acknowledgement of the entire patch.
> > For example, if a patch affects multiple subsystems and has an Acked-by: from
> > one subsystem maintainer then this usually indicates acknowledgement of just
> > the part which affects that maintainer's code.  Judgement should be used here.
> > When in doubt people should refer to the original discussion in the mailing
> > list archives.
> 
> And indeed, that is also true, especially for patch series affecting
> multiple subsystems.
> 
> However, while the meaning of Reviewed-by: is described very thoroughly
> in that same document, I cannot recall a single case of a patch series
> affecting a single driver that would get a Reviewed-by: _from the
> maintainer_.  Let alone a Reviewed-by: with a description of review
> depth.  Perhaps I have read too little threads (or the wrong ones) :)
> 
> With time, I have also grown to believe that one of the differences
> between Acked-by: and Reviewed-by: is that anyone interested can offer
> their Reviewed-by: while Acked-by: is reserved for driver maintainers.
> 
> Perhaps this is all material for a "falsehoods kernel developers believe
> about commit tags"-type article ;)

Thanks Michał for your thoughts/experience. My definitions above are based
largely on the recent kernel summit discussions and if that is not reflected in
the submitting patches document, it should be. I'm happy to propose a patch to
that affect and hopefully that will shake out any differences of opinion on
definitions.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center

Powered by blists - more mailing lists