Re: [PATCH] Avoid Wunused-but-set warning

[Jesper Juhl]

On Sun, 10 Jul 2011, Randy Dunlap wrote:

> On Mon, 11 Jul 2011 00:56:57 +0200 (CEST) Jesper Juhl wrote:
> 
> > On Sun, 10 Jul 2011, Randy Dunlap wrote:
> > 
> > > On Sun, 10 Jul 2011 12:49:20 -0400 Arnaud Lacombe wrote:
> > > 
> > > > Hi,
> > > > 
> > > > On Sun, Jul 10, 2011 at 12:28 PM, Pekka Enberg <penberg@xxxxxxxxxx> wrote:
> > > > > On Sun, Jul 10, 2011 at 7:24 PM, Arnaud Lacombe <lacombar@xxxxxxxxx> wrote:
> > > > >> Hi,
> > > > >>
> > > > >> On Sun, Jul 10, 2011 at 12:16 PM, Américo Wang <xiyou.wangcong@xxxxxxxxx> wrote:
> > > > >>> On Sun, Jul 10, 2011 at 12:24 AM, Raghavendra D Prabhu
> > > > >>> <rprabhu@xxxxxxxxxxx> wrote:
> > > > >>>> Hi,
> > > > >>>>    I am seeing Wunused-but-set warning while make nconfig.  Looks like
> > > > >>>>    active_menu is not used. Removing it fixes the warning.
> > > > >>>>
> > > > >>>> Signed-off-by: Raghavendra D Prabhu <rprabhu@xxxxxxxxxxx>
> > > > >>>
> > > > >>> Acked-by: WANG Cong <xiyou.wangcong@xxxxxxxxx>
> > > > >>>
> > > > >> Out of curiosity, what is your status to ACK such patch ?
> > > > >
> > > > > What kind of status do you need to ACK such a simple patch?
> > > > >
> > > > As per Documentation/SubmittingPatches:
> > > > 
> > > > <<
> > > > 13) When to use Acked-by: and Cc:
> > > > The Signed-off-by: tag indicates that the signer was involved in the
> > > > development of the patch, or that he/she was in the patch's delivery path.
> > > > 
> > > > If a person was not directly involved in the preparation or handling of a
> > > > patch but wishes to signify and record their approval of it then they can
> > > > arrange to have an Acked-by: line added to the patch's changelog.
> > > > 
> > > > Acked-by: is often used by the maintainer of the affected code when that
> > > > maintainer neither contributed to nor forwarded the patch.
> > > > >>
> > > > 
> > > > That said, it is not a strong requirement... unfortunately. So, let's
> > > > have some fun and go ACK thousand of trivial patch just to generate
> > > > traffic on the LKML and give myself self-importance :-)
> > > 
> > > Acked-by: is mostly used as a weak version of Reviewed-by:
> > > and the "definition" in SubmittingPatches is not accurate IMO.
> > > I.e., it can be used by anyone.
> > > 
> > 
> > Interesting. I was under the impression that Reviewed-by: was a weaker 
> > thing than Acked-by: - I certainly have been using it as such.
> > 
> > I've always interpreted Acked-by: as being something you could apply if 
> > you were the author, maintainer or other person with similar strong 
> > background knowledge of the code. Where Reviewed-by: could be used by 
> > anyone, as long as they had taken the time to read the patch and try and 
> > understand what was going on and the result/conclusion looked good.
> 
> I don't see it in SubmittingPatches, but there was some discussion at the
> time (IIRC!!) that Reviewed-by: indicates that you are willing to support/fix
> the patch if the patch author(s) disappear.  I.e., you are willing to take
> some ownership responsibility of the patch.
> 
> or I could be dreaming...
> 
I'm not going to claim that I recall all the discussion that went on at 
the time, there was quite a bit IIRC (and I'm too lazy to read up on all 
of it). But to me it seems to make sense that if you have strong knowledge 
of/involvement with the code being patched then you can offer your 
Acked-by: after reviewing the patch. If you don't have such 
knowledge/involvement but have nevertheless reviewed the code and found it 
to be OK, then you can signal that with a Reviewed-by:.

In any case, you can't expect people to base their Acked-by/Reviewed-by 
replies on some conclusion in some email thread that happened years ago 
but was never written down in some document in the repository. 
It is only reasonable to expect people to behave according to the rules 
laid out in SubmittingPatches and similar documents, and those rules 
currently seem to support my interpretation, not yours.

-- 
Jesper Juhl <jj@xxxxxxxxxxxxx>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.