2007-06-01 02:09:22

by Andrew Morton

[permalink] [raw]
Subject: [patch 1/1] document Acked-by:

From: Andrew Morton <[email protected]>

Explain what we use Acked-by: for, and how it differs from Signed-off-by:
Signed-off-by: Andrew Morton <[email protected]>
---

Documentation/SubmittingPatches | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)

diff -puN Documentation/SubmittingPatches~document-acked-by Documentation/SubmittingPatches
--- a/Documentation/SubmittingPatches~document-acked-by
+++ a/Documentation/SubmittingPatches
@@ -328,7 +328,20 @@ now, but you can do this to mark interna
point out some special detail about the sign-off.


-12) The canonical patch format
+12) When to use Acked-by:
+
+The Signed-off-by: tag implies 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 wrote, merged nor forwarded the patch themselves.
+
+
+13) The canonical patch format

The canonical patch subject line is:

_


2007-06-01 05:32:18

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Thu, 31 May 2007 19:09:10 PDT, [email protected] said:

> +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 wrote, merged nor forwarded the patch themselves.

Do we want to add verbiage saying that an Acked-By: is also useful when it
comes from somebody (likely the original reporter) who has actually tested the
patch?


Attachments:
(No filename) (226.00 B)

2007-06-01 06:10:59

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

[email protected] wrote:
> On Thu, 31 May 2007 19:09:10 PDT, [email protected] said:
>
>> +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 wrote, merged nor forwarded the patch themselves.
>
> Do we want to add verbiage saying that an Acked-By: is also useful when it
> comes from somebody (likely the original reporter) who has actually tested the
> patch?

I'd rather see a Tested-By: for that.

There is a difference between a maintainer ack and a tester ok.

-hpa

2007-06-01 10:53:19

by John Anthony Kazos Jr.

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

> >> +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 wrote, merged nor forwarded the patch themselves.
> >
> > Do we want to add verbiage saying that an Acked-By: is also useful when it
> > comes from somebody (likely the original reporter) who has actually tested the
> > patch?
>
> I'd rather see a Tested-By: for that.
>
> There is a difference between a maintainer ack and a tester ok.

Indeed. Acked-by: implies authority, and only very few people should be
able to do it. Namely, the only person who can ACK a patch is a person who
could also NACK a patch and expect it to actually be dropped. If I think a
patch is bad, I can say so, but as I have no authority, my statement would
be taken on merit alone, whereas Linus or Andrew or such could just NACK
it and move on without having to spew a blurb every time.

2007-06-01 17:27:36

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Thu, 31 May 2007 23:10:42 PDT, "H. Peter Anvin" said:
> [email protected] wrote:
> > On Thu, 31 May 2007 19:09:10 PDT, [email protected] said:
> >
> >> +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 wrote, merged nor forwarded the patch themselves.
> >
> > Do we want to add verbiage saying that an Acked-By: is also useful when it
> > comes from somebody (likely the original reporter) who has actually tested the
> > patch?
>
> I'd rather see a Tested-By: for that.
>
> There is a difference between a maintainer ack and a tester ok.

OK by me. Half the time when a -mm breaks for me, it's an obvious one-liner
I can S-o-b: myself, the other half the time somebody else has a fix that
I keep thinking I should stick *something* on once I confirm it's fixed.

Do Linus/Andrew/major maintainers want Tested-By:'s for patches?


Attachments:
(No filename) (226.00 B)

2007-06-01 17:35:53

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Fri, 01 Jun 2007 13:27:25 -0400
[email protected] wrote:

> Do Linus/Andrew/major maintainers want Tested-By:'s for patches?

I think it's very useful information to have. For a start, it tells
you who has the hardware and knows how to build a kernel. So if you're
making a change to a driver and want it tested, you can troll the file's
changelog looking for people who might be able to help.

A similar argument would apply to Reported-by:, which we possibly also
need. But it's all perhaps a bit much for poor kernel developers to
keep track of ;)

2007-06-01 18:14:56

by Dave Jones

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Thu, May 31, 2007 at 07:09:10PM -0700, Andrew Morton wrote:
> From: Andrew Morton <[email protected]>
>
> Explain what we use Acked-by: for, and how it differs from Signed-off-by:
> Signed-off-by: Andrew Morton <[email protected]>
> +
> +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: Dave Jones <[email protected]>

:-)

--
http://www.codemonkey.org.uk

2007-06-01 18:22:44

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Fri, 1 Jun 2007 14:14:14 -0400
Dave Jones <[email protected]> wrote:

> On Thu, May 31, 2007 at 07:09:10PM -0700, Andrew Morton wrote:
> > From: Andrew Morton <[email protected]>
> >
> > Explain what we use Acked-by: for, and how it differs from Signed-off-by:
> > Signed-off-by: Andrew Morton <[email protected]>
> > +
> > +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: Dave Jones <[email protected]>

What, no Tested-by: ?

2007-06-01 18:29:17

by Dave Jones

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Fri, Jun 01, 2007 at 11:22:25AM -0700, Andrew Morton wrote:
> On Fri, 1 Jun 2007 14:14:14 -0400
> Dave Jones <[email protected]> wrote:
>
> > On Thu, May 31, 2007 at 07:09:10PM -0700, Andrew Morton wrote:
> > > From: Andrew Morton <[email protected]>
> > >
> > > Explain what we use Acked-by: for, and how it differs from Signed-off-by:
> > > Signed-off-by: Andrew Morton <[email protected]>
> > > +
> > > +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: Dave Jones <[email protected]>
>
> What, no Tested-by: ?

Heh. Indeed. I think there's room for both fwiw.

Dave

--
http://www.codemonkey.org.uk

2007-06-01 19:27:17

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

"John Anthony Kazos Jr." <[email protected]> writes:

> Indeed. Acked-by: implies authority, and only very few people should be
> able to do it. Namely, the only person who can ACK a patch is a person who
> could also NACK a patch and expect it to actually be dropped. If I think a
> patch is bad, I can say so, but as I have no authority, my statement would
> be taken on merit alone, whereas Linus or Andrew or such could just NACK
> it and move on without having to spew a blurb every time.

Everyone always has some authority so everyone can ack or nack any
patch and I hope the action taken will always depend on merit
rather than person, especially if it's a technical issue and not
some style etc. thing.
--
Krzysztof Halasa

2007-06-01 19:38:06

by Scott Preece

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On 6/1/07, Krzysztof Halasa <[email protected]> wrote:
> "John Anthony Kazos Jr." <[email protected]> writes:
>
> > Indeed. Acked-by: implies authority, and only very few people should be
> > able to do it. Namely, the only person who can ACK a patch is a person who
> > could also NACK a patch and expect it to actually be dropped. If I think a
> > patch is bad, I can say so, but as I have no authority, my statement would
> > be taken on merit alone, whereas Linus or Andrew or such could just NACK
> > it and move on without having to spew a blurb every time.
>
> Everyone always has some authority so everyone can ack or nack any
> patch and I hope the action taken will always depend on merit
> rather than person, especially if it's a technical issue and not
> some style etc. thing.
> --
> Krzysztof Halasa
---

This is a question worth answering - is it rude to ack/nak a patch if
you're not a maintainer or otherwise known-to-be-trusted, or is it OK
for anyone to express an opinion? Andrew's patch text seems to imply
that it's generally OK.

scott

2007-06-01 20:01:26

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Fri, 1 Jun 2007 14:37:54 -0500
"Scott Preece" <[email protected]> wrote:

> On 6/1/07, Krzysztof Halasa <[email protected]> wrote:
> > "John Anthony Kazos Jr." <[email protected]> writes:
> >
> > > Indeed. Acked-by: implies authority, and only very few people should be
> > > able to do it. Namely, the only person who can ACK a patch is a person who
> > > could also NACK a patch and expect it to actually be dropped. If I think a
> > > patch is bad, I can say so, but as I have no authority, my statement would
> > > be taken on merit alone, whereas Linus or Andrew or such could just NACK
> > > it and move on without having to spew a blurb every time.
> >
> > Everyone always has some authority so everyone can ack or nack any
> > patch and I hope the action taken will always depend on merit
> > rather than person, especially if it's a technical issue and not
> > some style etc. thing.
> > --
> > Krzysztof Halasa
> ---
>
> This is a question worth answering - is it rude to ack/nak a patch if
> you're not a maintainer or otherwise known-to-be-trusted, or is it OK
> for anyone to express an opinion? Andrew's patch text seems to imply
> that it's generally OK.
>

I think saying "ack" or "nack" is generally a bit rude, and not very
useful.

It's better to just provide constructive, detailed technical comments and
from that it becomes pretty obvious to all parties whether or not the patch
has a future.

If you did properly provide that useful feedback then the "ack" or "nack" bit
becomes redundant.

2007-06-01 20:05:00

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Fri, 1 Jun 2007 13:00:24 -0700
Andrew Morton <[email protected]> wrote:

> On Fri, 1 Jun 2007 14:37:54 -0500
> "Scott Preece" <[email protected]> wrote:
>
> > On 6/1/07, Krzysztof Halasa <[email protected]> wrote:
> > > "John Anthony Kazos Jr." <[email protected]> writes:
> > >
> > > > Indeed. Acked-by: implies authority, and only very few people should be
> > > > able to do it. Namely, the only person who can ACK a patch is a person who
> > > > could also NACK a patch and expect it to actually be dropped. If I think a
> > > > patch is bad, I can say so, but as I have no authority, my statement would
> > > > be taken on merit alone, whereas Linus or Andrew or such could just NACK
> > > > it and move on without having to spew a blurb every time.
> > >
> > > Everyone always has some authority so everyone can ack or nack any
> > > patch and I hope the action taken will always depend on merit
> > > rather than person, especially if it's a technical issue and not
> > > some style etc. thing.
> > > --
> > > Krzysztof Halasa
> > ---
> >
> > This is a question worth answering - is it rude to ack/nak a patch if
> > you're not a maintainer or otherwise known-to-be-trusted, or is it OK
> > for anyone to express an opinion? Andrew's patch text seems to imply
> > that it's generally OK.
> >
>
> I think saying "ack" or "nack" is generally a bit rude, and not very
> useful.

err, make that "I think saying "nack" is generally a bit rude". An "ack"
is inoffensive and useful.

But frankly, I don't trust a simple "ack" much at all. It's the kernel
equivalent of "whoa, kewl!"

> It's better to just provide constructive, detailed technical comments and
> from that it becomes pretty obvious to all parties whether or not the patch
> has a future.

If the ack comes with some material of this nature then it becomes more believeable
that the Acker actually spent some time reading the code, and the ack becomes
more interesting.

It's quite common for experienced kernel developers to ack completely broken
patches.

2007-06-01 22:10:58

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

"Scott Preece" <[email protected]> writes:

> This is a question worth answering - is it rude to ack/nak a patch if
> you're not a maintainer or otherwise known-to-be-trusted, or is it OK
> for anyone to express an opinion? Andrew's patch text seems to imply
> that it's generally OK.

Every pair of eyes (or a single one) looking at the patch in question
is a good thing. I can't imagine why would one want to look at the
code if he/she can't ack or nak or otherwise comment it.
--
Krzysztof Halasa

2007-06-01 22:17:51

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

Krzysztof Halasa wrote:
> "Scott Preece" <[email protected]> writes:
>
>> This is a question worth answering - is it rude to ack/nak a patch if
>> you're not a maintainer or otherwise known-to-be-trusted, or is it OK
>> for anyone to express an opinion? Andrew's patch text seems to imply
>> that it's generally OK.
>
> Every pair of eyes (or a single one) looking at the patch in question
> is a good thing. I can't imagine why would one want to look at the
> code if he/she can't ack or nak or otherwise comment it.

I think the comment had to do with the concept that ACK/NAK implies
authority. If you're not the maintainer, it's rude to imply that you
are. Obvious, test reports (good or bad!) are always welcome.

-hpa

2007-06-01 22:42:47

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

"H. Peter Anvin" <[email protected]> writes:

> I think the comment had to do with the concept that ACK/NAK implies
> authority. If you're not the maintainer, it's rude to imply that you
> are. Obvious, test reports (good or bad!) are always welcome.

Well, I understand a test is a different thing, an experiment to
see if the patch works or not, while ack/etc. is just opinion
of someone who reads the patch without actually using it.

I think ack/etc doesn't, in any way, imply being the maintainer,
though it imply that the "acker" has actually read the code,
understands it, and believes it's correct (or not, and why).

If we want to differentiate between "authoritative" and
"non-authoritative" opinions (and the name and email address
aren't enough) then I think we need to state that explicite
(perhaps something like "Acked-by: FIRST M. LAST <addr>, XXX
subsystem maintainer" would suffice).
--
Krzysztof Halasa

2007-06-02 00:37:47

by John Anthony Kazos Jr.

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

> > I think the comment had to do with the concept that ACK/NAK implies
> > authority. If you're not the maintainer, it's rude to imply that you
> > are. Obvious, test reports (good or bad!) are always welcome.
>
> Well, I understand a test is a different thing, an experiment to
> see if the patch works or not, while ack/etc. is just opinion
> of someone who reads the patch without actually using it.
>
> I think ack/etc doesn't, in any way, imply being the maintainer,
> though it imply that the "acker" has actually read the code,
> understands it, and believes it's correct (or not, and why).
>
> If we want to differentiate between "authoritative" and
> "non-authoritative" opinions (and the name and email address
> aren't enough) then I think we need to state that explicite
> (perhaps something like "Acked-by: FIRST M. LAST <addr>, XXX
> subsystem maintainer" would suffice).

"Acked-by:" does not mean "I like this" but rather "I approve of this".
Someone who is not a maintainer is encouraged to speak of like and
dislike, in great detail, but has no position at all to approve or
disapprove of it going in.

If I put "Acked-by: John..." on a patch of any kind, even trivial, it
would look incredibly stupid, because I'm just some guy messing around
with the kernel. A tactful response to me doing that from any actual
kernel bigwig would be, "I appreciate your enthusiasm, but you are not
part of the kernel patch flow." Similarly, a tactful response to me
NACKing a patch would be, "I appreciate your concern, but you are in no
position to remove a patch from the stream. Your comments will be
considered and implemented or countered by an actual maintainer."

This is appropriate.

2007-06-02 00:56:40

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

"John Anthony Kazos Jr." <[email protected]> writes:

> "Acked-by:" does not mean "I like this" but rather "I approve of this".

I'd say it means "I acknowledge it". If you want to express
approval, why not use some sort of "Approved-by"?

> If I put "Acked-by: John..." on a patch of any kind, even trivial, it
> would look incredibly stupid, because I'm just some guy messing around
> with the kernel. A tactful response to me doing that from any actual
> kernel bigwig would be, "I appreciate your enthusiasm, but you are not
> part of the kernel patch flow." Similarly, a tactful response to me
> NACKing a patch would be, "I appreciate your concern, but you are in no
> position to remove a patch from the stream. Your comments will be
> considered and implemented or countered by an actual maintainer."
>
> This is appropriate.

You seem to know these things very well.
--
Krzysztof Halasa

2007-06-02 01:36:28

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Sat, 02 Jun 2007 00:10:46 +0200, Krzysztof Halasa said:
> "Scott Preece" <[email protected]> writes:
>
> > This is a question worth answering - is it rude to ack/nak a patch if
> > you're not a maintainer or otherwise known-to-be-trusted, or is it OK
> > for anyone to express an opinion? Andrew's patch text seems to imply
> > that it's generally OK.
>
> Every pair of eyes (or a single one) looking at the patch in question
> is a good thing. I can't imagine why would one want to look at the
> code if he/she can't ack or nak or otherwise comment it.

I'd be the *first* to admit that my kernel-foo isn't perfect, and sometimes I'm
right and sometimes I'm wrong when I review somebody else's code. I certainly
*hope* that nobody's taking my review as anything more authoritative than "an
actual maintainer might want to look at this".

On the other hand, we don't need a Foo-By: tag for "or otherwise comment".

Phrased differently, if I haven't stuck a "Signed-off-by:" or "Tested-By:"
on it, I'm by default only commenting. The code submitter can decide I'm right
and fix and resubmit, the maintainer can decide I'm right and toss a NAK. Or
they can both decide I'm full of it and hit the Delete key..


Attachments:
(No filename) (226.00 B)

2007-06-02 13:34:38

by Andev

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On 6/2/07, Andrew Morton <[email protected]> wrote:

> It's quite common for experienced kernel developers to ack completely broken
> patches.

common!!

is'nt that a bit too ...

2007-06-02 14:11:50

by Adrian Bunk

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Thu, May 31, 2007 at 07:09:10PM -0700, [email protected] wrote:
> From: Andrew Morton <[email protected]>
>
> Explain what we use Acked-by: for, and how it differs from Signed-off-by:
> Signed-off-by: Andrew Morton <[email protected]>
> ---
>
> Documentation/SubmittingPatches | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff -puN Documentation/SubmittingPatches~document-acked-by Documentation/SubmittingPatches
> --- a/Documentation/SubmittingPatches~document-acked-by
> +++ a/Documentation/SubmittingPatches
> @@ -328,7 +328,20 @@ now, but you can do this to mark interna
> point out some special detail about the sign-off.
>
>
> -12) The canonical patch format
> +12) When to use Acked-by:
> +
> +The Signed-off-by: tag implies that the signer was involved in the development
> +of the patch, or that he/she was in the patch's delivery path.

The last part should be dropped: If "he/she was in the patch's delivery
path", a Signed-off-by: tag is required.

> +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 wrote, merged nor forwarded the patch themselves.

"merged" seems to be superfluous if you also mention "forwarded".

> +13) The canonical patch format
>
> The canonical patch subject line is:

Please mention explicitely whether Acked-by: this now considered a
formal tag like Signed-off-by:

IOW, if a maintainer says "fine with me", can I translate this to an
Acked-by: line, or do I now have to ask for an explicit Acked-by: line?

Oh, and that's not a theoretical question, this is a result of a recent
flamewar^Wdiscussion on this list...

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2007-06-02 17:18:43

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:


On Jun 1 2007 14:28, Dave Jones wrote:
> > > On Thu, May 31, 2007 at 07:09:10PM -0700, Andrew Morton wrote:
> > > > From: Andrew Morton <[email protected]>
> > > >
> > > > Explain what we use Acked-by: for, and how it differs from Signed-off-by:
> > > > Signed-off-by: Andrew Morton <[email protected]>
> > > > +
> > > > +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: Dave Jones <[email protected]>
> >
> > What, no Tested-by: ?
>
>Heh. Indeed. I think there's room for both fwiw.

Too verbose. Suggest a typedef.

Signed-off-and-tested-by: Foo J. Bar <addy@corps>


Jan
--

2007-06-02 17:21:22

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:


On Jun 2 2007 19:04, debian developer wrote:
> On 6/2/07, Andrew Morton <[email protected]> wrote:
>
>> It's quite common for experienced kernel developers to ack completely
>> broken
>> patches.
>
> common!!
>
> is'nt that a bit too ...

too nack?


Jan
--

2007-06-02 17:47:31

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Sat, 2 Jun 2007 16:11:45 +0200 Adrian Bunk <[email protected]> wrote:

> On Thu, May 31, 2007 at 07:09:10PM -0700, [email protected] wrote:
> > From: Andrew Morton <[email protected]>
> >
> > Explain what we use Acked-by: for, and how it differs from Signed-off-by:
> > Signed-off-by: Andrew Morton <[email protected]>
> > ---
> >
> > Documentation/SubmittingPatches | 15 ++++++++++++++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff -puN Documentation/SubmittingPatches~document-acked-by Documentation/SubmittingPatches
> > --- a/Documentation/SubmittingPatches~document-acked-by
> > +++ a/Documentation/SubmittingPatches
> > @@ -328,7 +328,20 @@ now, but you can do this to mark interna
> > point out some special detail about the sign-off.
> >
> >
> > -12) The canonical patch format
> > +12) When to use Acked-by:
> > +
> > +The Signed-off-by: tag implies that the signer was involved in the development
> > +of the patch, or that he/she was in the patch's delivery path.
>
> The last part should be dropped: If "he/she was in the patch's delivery
> path", a Signed-off-by: tag is required.

I don't get you. Isn't that already what the text says?

> > +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 wrote, merged nor forwarded the patch themselves.
>
> "merged" seems to be superfluous if you also mention "forwarded".

OK

> > +13) The canonical patch format
> >
> > The canonical patch subject line is:
>
> Please mention explicitely whether Acked-by: this now considered a
> formal tag like Signed-off-by:

OK.

> IOW, if a maintainer says "fine with me", can I translate this to an
> Acked-by: line, or do I now have to ask for an explicit Acked-by: line?

I do that often. It's useful information. If person X sends an fbdev
patch and Tony says "whoa, neat" and I send the patch to Linus then Linus could
well think "wtf, Andrew doesn't know anything about fbdev". So I do s/whoa
neat/Acked-by:/ to tell the world that someone who knows something has
looked at the change.

> Oh, and that's not a theoretical question, this is a result of a recent
> flamewar^Wdiscussion on this list...

yeah, well, what isn't ;)

The person whose Acked-by: I added will get a copy of the added-to-mm email
so if they didn't want that acked-by added then they get a chance to remove
it again.


From: Andrew Morton <[email protected]>

Explain what we use Acked-by: for, and how it differs from Signed-off-by:

Acked-by: Dave Jones <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

Documentation/SubmittingPatches | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)

diff -puN Documentation/SubmittingPatches~document-acked-by Documentation/SubmittingPatches
--- a/Documentation/SubmittingPatches~document-acked-by
+++ a/Documentation/SubmittingPatches
@@ -340,8 +340,32 @@ now, but you can do this to mark interna
point out some special detail about the sign-off.


+13) When to use Acked-by:

-13) The canonical patch format
+The Signed-off-by: tag implies 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 themselves.
+
+Acked-by: is not as formal as Signed-off-by:. It is a record that the acker
+has at least reviewed the patch and has indicated acceptance. Hence patch
+mergers will sometimes manually covert an acker's "yep, looks good to me" into
+an Acked-by:.
+
+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.
+
+
+14) The canonical patch format

The canonical patch subject line is:

_

2007-06-02 17:55:54

by Adrian Bunk

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Sat, Jun 02, 2007 at 10:47:00AM -0700, Andrew Morton wrote:
> On Sat, 2 Jun 2007 16:11:45 +0200 Adrian Bunk <[email protected]> wrote:
>...
> > > -12) The canonical patch format
> > > +12) When to use Acked-by:
> > > +
> > > +The Signed-off-by: tag implies that the signer was involved in the development
> > > +of the patch, or that he/she was in the patch's delivery path.
> >
> > The last part should be dropped: If "he/she was in the patch's delivery
> > path", a Signed-off-by: tag is required.
>
> I don't get you. Isn't that already what the text says?
>...

/me blind

> +Acked-by: is often used by the maintainer of the affected code when that
> +maintainer neither contributed to nor forwarded the patch themselves.
> +
> +Acked-by: is not as formal as Signed-off-by:. It is a record that the acker
> +has at least reviewed the patch and has indicated acceptance. Hence patch
> +mergers will sometimes manually covert an acker's "yep, looks good to me" into
> +an Acked-by:.
> +
> +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.
> +
> +
> +14) The canonical patch format
>...

Looks good to me.

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2007-06-02 18:00:27

by John Anthony Kazos Jr.

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

> > > > > Explain what we use Acked-by: for, and how it differs from Signed-off-by:
> > > > > Signed-off-by: Andrew Morton <[email protected]>
> > > > > +
> > > > > +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: Dave Jones <[email protected]>
> > >
> > > What, no Tested-by: ?
> >
> >Heh. Indeed. I think there's room for both fwiw.
>
> Too verbose. Suggest a typedef.
>
> Signed-off-and-tested-by: Foo J. Bar <addy@corps>

Signed-off-by: should imply Tested-by:, with the exception of the final
Signed-off-by: when it's merged into a tree. Tested-by:, if it really is
necessary or useful, should be reserved for only those who test something
but weren't involved in its development. Adding it to the tag is
unnecessary unless somebody thinks there's a serious problem with untested
patches being introduced by first-hand maintainers that a forced reminder
would remedy.

2007-06-02 19:06:46

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Sat, Jun 02, 2007 at 02:00:00PM -0400, John Anthony Kazos Jr. wrote:
> > > > > > Explain what we use Acked-by: for, and how it differs from Signed-off-by:
> > > > > > Signed-off-by: Andrew Morton <[email protected]>
> > > > > > +
> > > > > > +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: Dave Jones <[email protected]>
> > > >
> > > > What, no Tested-by: ?
> > >
> > >Heh. Indeed. I think there's room for both fwiw.
> >
> > Too verbose. Suggest a typedef.
> >
> > Signed-off-and-tested-by: Foo J. Bar <addy@corps>
>
> Signed-off-by: should imply Tested-by:, with the exception of the final
> Signed-off-by: when it's merged into a tree.
Subsystem maintainers cannot test each and every submission.
Sometimes due to lack of HW at other times simply due to lack of time.

Signed-off-by is exactly one thing - a way to show the path
a patch take. Then people on the path may have done more or less review/test.

Lot's of people confuses signed-of-by with acked-by btw - but this is waht this
patch should correct.

Sam

2007-06-02 19:35:16

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Sat, 02 Jun 2007 19:04:29 +0530, debian developer said:
> On 6/2/07, Andrew Morton <[email protected]> wrote:
>
> > It's quite common for experienced kernel developers to ack completely broken
> > patches.
>
> common!!
>
> is'nt that a bit too ...

Lots of code looks totally reasonable to a kernel coder, except it doesn't
actually work when run, or doesn't actually build for a common architecture.
Let's face it - if I hit a bug on my Dell Latitude in a -mm kernel, our
current depth of testers means that I'm probably the only person who's
likely to test the fix before it goes uptream to Linus as a "probably works".


Attachments:
(No filename) (226.00 B)

2007-06-03 00:23:16

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

Andrew Morton <[email protected]> writes:

> I do that often. It's useful information. If person X sends an fbdev
> patch and Tony says "whoa, neat" and I send the patch to Linus then
> Linus could
> well think "wtf, Andrew doesn't know anything about fbdev". So I do s/whoa
> neat/Acked-by:/ to tell the world that someone who knows something has
> looked at the change.

Makes sense.

> +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.

I'd add, just to be explicit, that acked-by does not necessarily mean
the acker is a maintainer of any subsystem the patch touches, or
generally maintainer of any code.

Unless it isn't true, of course, i.e., unless we really want to limit
expressing non-maintainers opinion in form of ack and nak.
--
Krzysztof Halasa

2007-06-03 02:57:51

by Scott Preece

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On 6/2/07, Andrew Morton <[email protected]> wrote:
> ...
> +The Signed-off-by: tag implies that the signer was involved in the development
---

Change "implies" to "indicates" - it's an explicit statement, not an
implication.

---
> +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 themselves.
---

This using plural pronouns for indefinite gender leaves one in vague
territory, but I think "themself" would be better than "themselves,
since "maintainer" is singular.

---
> +
> +Acked-by: is not as formal as Signed-off-by:. It is a record that the acker
> +has at least reviewed the patch and has indicated acceptance. Hence patch
> +mergers will sometimes manually covert an acker's "yep, looks good to me" into
---

"covert" -> "convert"

---
> +an Acked-by:.
> +
> +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.
> +
> +
> +14) The canonical patch format
>
> The canonical patch subject line is:

2007-06-03 04:09:41

by Randy Dunlap

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Sat, 2 Jun 2007 19:57:41 -0700 Scott Preece wrote:

> On 6/2/07, Andrew Morton <[email protected]> wrote:
> > ...
> > +The Signed-off-by: tag implies that the signer was involved in the development
> ---
>
> Change "implies" to "indicates" - it's an explicit statement, not an
> implication.
>
> ---
> > +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 themselves.
> ---
>
> This using plural pronouns for indefinite gender leaves one in vague
> territory, but I think "themself" would be better than "themselves,
> since "maintainer" is singular.

ugh. :(
not-acked-by: /me

I would like to say "no such word," but I googled it and found lots of
references to it, most of which say "sub-standard," "nonstandard,"
"mythical," "vulgar," or "colloq." But my search was not exhaustive.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

http://www.urbandictionary.com/define.php?term=themself
http://itre.cis.upenn.edu/~myl/languagelog/archives/004285.html
http://en.wiktionary.org/wiki/themself
http://www.allwords.com/word-themself.html

2007-06-03 04:16:14

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On Sat, 2 Jun 2007 21:06:14 -0700 Randy Dunlap <[email protected]> wrote:

> On Sat, 2 Jun 2007 19:57:41 -0700 Scott Preece wrote:
>
> > On 6/2/07, Andrew Morton <[email protected]> wrote:
> > > ...
> > > +The Signed-off-by: tag implies that the signer was involved in the development
> > ---
> >
> > Change "implies" to "indicates" - it's an explicit statement, not an
> > implication.
> >
> > ---
> > > +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 themselves.
> > ---
> >
> > This using plural pronouns for indefinite gender leaves one in vague
> > territory, but I think "themself" would be better than "themselves,
> > since "maintainer" is singular.
>
> ugh. :(
> not-acked-by: /me
>

I just deleted it ;)

"neither contributed to nor forwarded the patch."

2007-06-03 18:31:40

by Scott Preece

[permalink] [raw]
Subject: Re: [patch 1/1] document Acked-by:

On 6/2/07, Andrew Morton <[email protected]> wrote:
> On Sat, 2 Jun 2007 21:06:14 -0700 Randy Dunlap <[email protected]> wrote:
>
> > On Sat, 2 Jun 2007 19:57:41 -0700 Scott Preece wrote:
> >
> > > On 6/2/07, Andrew Morton <[email protected]> wrote:
> > > > ...
> > > > +The Signed-off-by: tag implies that the signer was involved in the development
> > > ---
> > >
> > > Change "implies" to "indicates" - it's an explicit statement, not an
> > > implication.
> > >
> > > ---
> > > > +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 themselves.
> > > ---
> > >
> > > This using plural pronouns for indefinite gender leaves one in vague
> > > territory, but I think "themself" would be better than "themselves,
> > > since "maintainer" is singular.
> >
> > ugh. :(
> > not-acked-by: /me
> >
>
> I just deleted it ;)
>
> "neither contributed to nor forwarded the patch."
>
---

Wise as usual - much better!

scott