Problematic culture around Signed-off-by

Problematic culture around Signed-off-by

[Ian Molton]

Hi folks,

I've been away from kernel development for a bit, but I've returned and
I'm troubled by what seems to be an entrenched and widespread (IMO)
misuse of the "Signed-off-by:" in commits.

I've now either been asked to sign off RFC quality patches "because its
quicker" on more than one occasion in the last week or so, and I've seen
others signing off code which clearly has no hope of going anywhere near
the kernel. (eg. // commented out lines)

I was of the impression that Signed-off-by: was intended to be used on
essentially *finished* commits, indicating both readiness for inclusion
upstream and ones ownership of the copyright.

Even if the intent is *purely* a copyright isue, Signing off
*everything* surely makes it far too easy for people to get junk into
the kernel.

I've actually been deliberately NOT signing off my code because I *DONT*
want it surreptitiously applied with my name on it unless its 100% spot on.

WTH is going on? How can we fix this?

Is Signed-off-by: a lost cause for code quality?

Do we need to add a

Ready-for-mainline: {yes,no} field as well?

Have I just missed something? Maybe I'm just in a minority here? Whats
going on?

-Ian

Re: Problematic culture around Signed-off-by

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]

Hi!

> I've been away from kernel development for a bit, but I've returned and
> I'm troubled by what seems to be an entrenched and widespread (IMO)
> misuse of the "Signed-off-by:" in commits.
> 
> I've now either been asked to sign off RFC quality patches "because its
> quicker" on more than one occasion in the last week or so, and I've seen
> others signing off code which clearly has no hope of going anywhere near
> the kernel. (eg. // commented out lines)
> 
> I was of the impression that Signed-off-by: was intended to be used on
> essentially *finished* commits, indicating both readiness for inclusion
> upstream and ones ownership of the copyright.
> 
> Even if the intent is *purely* a copyright isue, Signing off
> *everything* surely makes it far too easy for people to get junk into
> the kernel.

Well, maintainers should not apply obvious junk to their trees,
signed-off or not.

I normally sign-off everything... because getting patch without
sign-off is nasty. If maintainer gets unclean, but signed-off patch,
he can just clean it up, add his sign-off and continue normally.

That may or may not be allowed if patch is not signed-off. (We are in
lawyer teritory now.)

So I'd recommend signing everything, and if patch is considered "not
ready", make it clear in some other way.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: Problematic culture around Signed-off-by

[Adam Borowski]

On Sun, Jul 30, 2017 at 08:52:36PM +0200, Pavel Machek wrote:
> > I've been away from kernel development for a bit, but I've returned and
> > I'm troubled by what seems to be an entrenched and widespread (IMO)
> > misuse of the "Signed-off-by:" in commits.
> > 
> > I've now either been asked to sign off RFC quality patches "because its
> > quicker" on more than one occasion in the last week or so, and I've seen
> > others signing off code which clearly has no hope of going anywhere near
> > the kernel. (eg. // commented out lines)
> > 
> > I was of the impression that Signed-off-by: was intended to be used on
> > essentially *finished* commits, indicating both readiness for inclusion
> > upstream and ones ownership of the copyright.
> > 
> > Even if the intent is *purely* a copyright isue, Signing off
> > *everything* surely makes it far too easy for people to get junk into
> > the kernel.
> 
> I normally sign-off everything... because getting patch without
> sign-off is nasty. If maintainer gets unclean, but signed-off patch,
> he can just clean it up, add his sign-off and continue normally.

Yet there are cases with known but unobvious breakage (see below).

> That may or may not be allowed if patch is not signed-off. (We are in
> lawyer teritory now.)
> 
> So I'd recommend signing everything, and if patch is considered "not
> ready", make it clear in some other way.

I think it'd be much better if you could suggest a new marker.  Something
like "Copyright-but-not-Readiness-Signed-off-by:", "RFC-Signed-off-by:",
"WIP-Signed-off-by:", etc.

And having a common, agreed upon marker would avoid confusion.

For example, I'm currently messing with a patch set that appears to give a
nice speed-up for the O_PONIES issue, but it messes with parts of the kernel
I'm really unfamiliar with.  Moreover, I know of data-loss scenarios in my
current version (I have naive ideas for a fix), but these scenarios are
irrelevant for a RFC.  Thus, it's imperative these patches do not get into
any non-experimental tree -- yet it's also likely that, if the concept
proves as good as it seems, that one of kernel devs with more skill would
take my work and beat it into sanity.

Cases like this just scream for a "Here-be-Dragons-but-Signed-off-Copyright-
Wise:" line.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ What Would Jesus Do, MUD/MMORPG edition:
⣾⠁⢰⠒⠀⣿⡁ • multiplay with an admin char to benefit your mortal
⢿⡄⠘⠷⠚⠋⠀ • abuse item cloning bugs (the five fishes + two breads affair)
⠈⠳⣄⠀⠀⠀⠀ • use glitches to walk on water

Re: Problematic culture around Signed-off-by

[Kirill A. Shutemov]

On Mon, Jul 31, 2017 at 03:34:11PM +0200, Adam Borowski wrote:
> On Sun, Jul 30, 2017 at 08:52:36PM +0200, Pavel Machek wrote:
> > > I've been away from kernel development for a bit, but I've returned and
> > > I'm troubled by what seems to be an entrenched and widespread (IMO)
> > > misuse of the "Signed-off-by:" in commits.
> > > 
> > > I've now either been asked to sign off RFC quality patches "because its
> > > quicker" on more than one occasion in the last week or so, and I've seen
> > > others signing off code which clearly has no hope of going anywhere near
> > > the kernel. (eg. // commented out lines)
> > > 
> > > I was of the impression that Signed-off-by: was intended to be used on
> > > essentially *finished* commits, indicating both readiness for inclusion
> > > upstream and ones ownership of the copyright.
> > > 
> > > Even if the intent is *purely* a copyright isue, Signing off
> > > *everything* surely makes it far too easy for people to get junk into
> > > the kernel.
> > 
> > I normally sign-off everything... because getting patch without
> > sign-off is nasty. If maintainer gets unclean, but signed-off patch,
> > he can just clean it up, add his sign-off and continue normally.
> 
> Yet there are cases with known but unobvious breakage (see below).
> 
> > That may or may not be allowed if patch is not signed-off. (We are in
> > lawyer teritory now.)
> > 
> > So I'd recommend signing everything, and if patch is considered "not
> > ready", make it clear in some other way.
> 
> I think it'd be much better if you could suggest a new marker.  Something
> like "Copyright-but-not-Readiness-Signed-off-by:", "RFC-Signed-off-by:",
> "WIP-Signed-off-by:", etc.

I use (and saw other people used) "Not-Yet-Signed-off-by:" for this
purpose.

-- 
 Kirill A. Shutemov

Re: Problematic culture around Signed-off-by

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2495 bytes --]

On Mon 2017-07-31 16:44:49, Kirill A. Shutemov wrote:
> On Mon, Jul 31, 2017 at 03:34:11PM +0200, Adam Borowski wrote:
> > On Sun, Jul 30, 2017 at 08:52:36PM +0200, Pavel Machek wrote:
> > > > I've been away from kernel development for a bit, but I've returned and
> > > > I'm troubled by what seems to be an entrenched and widespread (IMO)
> > > > misuse of the "Signed-off-by:" in commits.
> > > > 
> > > > I've now either been asked to sign off RFC quality patches "because its
> > > > quicker" on more than one occasion in the last week or so, and I've seen
> > > > others signing off code which clearly has no hope of going anywhere near
> > > > the kernel. (eg. // commented out lines)
> > > > 
> > > > I was of the impression that Signed-off-by: was intended to be used on
> > > > essentially *finished* commits, indicating both readiness for inclusion
> > > > upstream and ones ownership of the copyright.
> > > > 
> > > > Even if the intent is *purely* a copyright isue, Signing off
> > > > *everything* surely makes it far too easy for people to get junk into
> > > > the kernel.
> > > 
> > > I normally sign-off everything... because getting patch without
> > > sign-off is nasty. If maintainer gets unclean, but signed-off patch,
> > > he can just clean it up, add his sign-off and continue normally.
> > 
> > Yet there are cases with known but unobvious breakage (see below).

Yes, so you point up the breakage in the changelog...

> > > That may or may not be allowed if patch is not signed-off. (We are in
> > > lawyer teritory now.)
> > > 
> > > So I'd recommend signing everything, and if patch is considered "not
> > > ready", make it clear in some other way.
> > 
> > I think it'd be much better if you could suggest a new marker.  Something
> > like "Copyright-but-not-Readiness-Signed-off-by:", "RFC-Signed-off-by:",
> > "WIP-Signed-off-by:", etc.
> 
> I use (and saw other people used) "Not-Yet-Signed-off-by:" for this
> purpose.

As I tried to explain, that is problematic.

If I fix the patch, how do I submit it myself?

But you are free to use Subject: [Not ready], or just sprinkle code
with // comments...

Anyway, applying not-ready patch is not something I usually seen
happening. OTOH, not applying patches that were ready months ago is
quite common :-).

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]