checkpatch: add fix and improve warning msg for Non-standard signature
PATCH v5 1/1 checkpatch: add fix and improve warning msg for Non-standard signature
Currently, checkpatch.pl warns for BAD_SIGN_OFF on non-standard signature styles.
This warning occurs because of incorrect use of signature tags, e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect signature tags, which may seem correct, but are not standard:
1) Requested-by (count: 48) => Suggested-by
Rationale: In an open-source project, there are no 'requests', just 'suggestions' to convince a maintainer to accept your patch
2) Co-authored-by (count: 43) => Co-developed-by
Rationale: Co-developed-by and Co-authored-by are synonyms
3) Analyzed-by (count: 22) / Analysed-by (count: 5) => Co-developed-by
Rationale: Analyzing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code
4) Improvements-by (count: 19) => Co-developed-by
5) Noticed-by (count: 11) => Reported-by
6) Inspired-by (count: 11) => Suggested-by
7) Verified-by (count: 8) => Tested-by
Rationale: Used by a single user. On reading mailing list, it seems Tested-by might be a suitable alternative
8) Okay-ished-by (count: 8) => Acked-by
Rationale: Used by a single user. On reading mailing list, it seems Acked-by must be suitable alternative
9) Acked-for-MFD-by (count: 6) => Acked-by
10) Reviewed-off-by (count: 5) => Reviewed-by
11) Proposed-by (count: 5) => Suggested-by
Rationale: On observing the mailing list, this tag is always used for a maintainer. It seems that the changes might have been suggested by them and the tag is used as acknowledgment for the same
12) Fixed-by (count: 3) => Co-developed-by
Rationale: Fixing bug is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code
13) Pointed-out-by (count: 3) / Pointed-at-by (count: 2) => Suggested-by
Rationale: The tags are used for maintainers. It seems that the changes might have been suggested by them and the tag is used as acknowledgment for the same
E.g., Pointed-at-by: Greg Kroah-Hartman <[email protected]>
14) Suggestions-by (count: 3) => Suggested-by
Provide a fix by replacing the non-standard signature with its standard equivalent.
Also, improve warning messages correspondingly, providing suitable rationale to the user for the suggestion made.
---
applies on next-20201120 and my last patch at Link: https://lore.kernel.org/linux-kernel-mentees/[email protected]/T/#u
changes in v2: replace commit specific example with brief evaluation
changes in v3: provide rationale to users for every signature tag suggestion; modify commit message describing arrival to conclusion in a structured way
changes in v4: modify rationale for certain suggestions
changes in v5: remove the tag deletion suggestions, ie "Generated-by" and "Celebrated-by"; rebase on last accepted changes; modify commit message
scripts/checkpatch.pl | 73 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-)
scripts/checkpatch.pl +72 -1
@@ -563,6 +563,72 @@ sub find_standard_signature { return ""; } +our %standard_signature_fix = ( + "Requested-by:" => { + suggestion => "Suggested-by:", + rationale => "In an open-source project, there are no 'requests', just 'suggestions' to convince a maintainer to accept your patch", + }, + "Co-authored-by:" => { + suggestion => "Co-developed-by:", + rationale => "Co-developed-by is the standard signature tag to attribute multiple authors for a patch", + }, + "Analyzed-by:" => { + suggestion => "Co-developed-by:", + rationale => "Analyzing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code", + }, + "Analysed-by:" => { + suggestion => "Co-developed-by:", + rationale => "Analysing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code", + }, + "Improvements-by:" => { + suggestion => "Co-developed-by:", + rationale => "Performing improvements are a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code", + }, + "Noticed-by:" => { + suggestion => "Reported-by:", + rationale => "Reported-by is the standard signature tag for acknowledging user who noticed or reported any bug(s)", + }, + "Inspired-by:" => { + suggestion => "Suggested-by:", + rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions", + }, + "Verified-by:" => { + suggestion => "Tested-by:", + rationale => "Tested-by is the standard signature tag to attribute user for verifying/testing the patch", + }, + "Okay-ished-by:" => { + suggestion => "Acked-by:", + rationale => "Acked-by is the standard signature tag for recording one's approval", + }, + "Acked-for-MFD-by:" => { + suggestion => "Acked-by:", + rationale => "Acked-by is the standard signature tag for recording one's approval", + }, + "Reviewed-off-by:" => { + suggestion => "Reviewed-by:", + rationale => "Reviewed-by is the standard signature tag to indicate that the patch has been reviewed", + }, + "Proposed-by:" => { + suggestion => "Suggested-by:", + rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions", + }, + "Fixed-by:" => { + suggestion => "Co-developed-by:", + rationale => "Fixing bug is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code", + }, + "Pointed-out-by:" => { + suggestion => "Suggested-by:", + rationale => "Pointing out certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine", + }, + "Pointed-at-by:" => { + suggestion => "Suggested-by:", + rationale => "Pointing at certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine", + }, + "Suggestions-by:" => { + suggestion => "Suggested-by:", + rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions", + }, +); our @typeListMisordered = ( qr{char\s+(?:un)?signed},
@@ -2832,12 +2898,17 @@ sub process { if ($sign_off !~ /$signature_tags/) { my $suggested_signature = find_standard_signature($sign_off); + my $rationale = ""; + if (exists($standard_signature_fix{$sign_off})) { + $suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'}; + $rationale = $standard_signature_fix{$sign_off}{'rationale'}; + } if ($suggested_signature eq "") { WARN("BAD_SIGN_OFF", "Non-standard signature: $sign_off\n" . $herecurr); } else { if (WARN("BAD_SIGN_OFF", - "Non-standard signature: '$sign_off' - perhaps '$suggested_signature'?\n" . $herecurr) && + "Non-standard signature: '$sign_off' - perhaps '$suggested_signature'? $rationale\n" . $herecurr) && $fix) { $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/; }
Signature · 6 lines
2.17.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Joe Perches
maintainerQuoted 6 lines · Aditya Srivastava
Currently, checkpatch.pl warns for
BAD_SIGN_OFFon non-standard signature styles.This warning occurs because of incorrect use of signature tags, e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect signature tags, which may seem correct, but are not standard:
I'm not a fan of this patch.
There is already a "non-standard" signature warning for all of these cases since 2012, predating the range of this retrospective evaluation by over 5 years and yet these existing commits have been accepted.
The value in actual standardization and effectively requiring specific signature style tags is quite low.
Anyone that signed a thing a particular way should be free to sign the thing as they choose.
Most of these warnings would also still be in the tree in the future in new patches as running checkpatch without it emitting a message of any type isn't a requirement nor should checkpatch use actually be required workflow.
Quoted 19 lines · Joe Perches
Quoted 6 lines · Aditya Srivastava
Currently, checkpatch.pl warns for
BAD_SIGN_OFFon non-standard signature styles.This warning occurs because of incorrect use of signature tags, e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect signature tags, which may seem correct, but are not standard:
I'm not a fan of this patch.
There is already a "non-standard" signature warning for all of these cases since 2012, predating the range of this retrospective evaluation by over 5 years and yet these existing commits have been accepted.
The value in actual standardization and effectively requiring specific signature style tags is quite low.
Quoted 8 lines
Anyone that signed a thing a particular way should be free to sign the thing as they choose.
Most of these warnings would also still be in the tree in the future in new patches as running checkpatch without it emitting a message of any type isn't a requirement nor should checkpatch use actually be required workflow.
Can we scale this fixing feature down to the very obvious synonyms that simply do not add anything but confusion?
Such as for those four here:
Co-authored-by (count: 43) => Co-developed-by
Reviewed-off-by (count: 5) => Reviewed-by
Proposed-by (count: 5) => Suggested-by
Suggestions-by (count: 3) => Suggested-by
Then, we can probably also drop the rationale because it is pretty clear.
Of course, the impact might be really zero, given that it is unclear if those authors did actually ever run checkpatch in the first place.
Joe, if you see no value in even such a minimal fix feature, let us drop that idea and move on. There are enough other things to work on.
Lukas
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Joe Perches
maintainerQuoted 29 lines · Lukas Bulwahn
Quoted 19 lines · Joe Perches
Quoted 6 lines · Aditya Srivastava
Currently, checkpatch.pl warns for
BAD_SIGN_OFFon non-standard signature styles.This warning occurs because of incorrect use of signature tags, e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect signature tags, which may seem correct, but are not standard:
I'm not a fan of this patch.
There is already a "non-standard" signature warning for all of these cases since 2012, predating the range of this retrospective evaluation by over 5 years and yet these existing commits have been accepted.
The value in actual standardization and effectively requiring specific signature style tags is quite low.
Quoted 8 lines
Anyone that signed a thing a particular way should be free to sign the thing as they choose.
Most of these warnings would also still be in the tree in the future in new patches as running checkpatch without it emitting a message of any type isn't a requirement nor should checkpatch use actually be required workflow.
Quoted 6 lines
Can we scale this fixing feature down to the very obvious synonyms that simply do not add anything but confusion?
Such as for those four here:
Co-authored-by (count: 43) => Co-developed-by
I've never been a big fan of "Co-developed-by" as a signature tag, but a "this should be that" here could be ok.
Quoted 1 line
Reviewed-off-by (count: 5) => Reviewed-by
I don't see value. If no one notices a BAD_SIGN_OFF for the Reviewed-off-by:, I doubt this would add anything.
Quoted 2 lines
Proposed-by (count: 5) => Suggested-by
Suggestions-by (count: 3) => Suggested-by
Suggestions-by is not suggested-by as these suggestions could have been in response to an initial patch proposal and the author could have incorporated those suggestions.
Quoted 7 lines
Then, we can probably also drop the rationale because it is pretty clear.
Of course, the impact might be really zero, given that it is unclear if those authors did actually ever run checkpatch in the first place.
Joe, if you see no value in even such a minimal fix feature, let us drop that idea and move on. There are enough other things to work on.
Maybe only add the Co-authored-by: -> Co-developed-by: check.
But IMO: none of this is particularly useful.
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH 1/1 checkpatch: add fix for non-standard signature - co-authored-by
Currently, checkpatch.pl warns for BAD_SIGN_OFF on the usage of non-standard signatures.
An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
Provide a fix by suggesting users with this signature alternative and replacing.
--- scripts/checkpatch.pl | 3 +++ 1 file changed, 3 insertions(+)
scripts/checkpatch.pl +3 -0
@@ -2832,6 +2832,9 @@ sub process { if ($sign_off !~ /$signature_tags/) { my $suggested_signature = find_standard_signature($sign_off); + if ($sign_off =~ /co-authored-by:/i) { + $suggested_signature = "Co-developed-by:"; + } if ($suggested_signature eq "") { WARN("BAD_SIGN_OFF", "Non-standard signature: $sign_off\n" . $herecurr);
Signature · 6 lines
2.17.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Joe Perches
maintainerRe: [PATCH] checkpatch: add fix for non-standard signature - co-authored-by
Quoted 7 lines · Aditya Srivastava
Currently, checkpatch.pl warns for
BAD_SIGN_OFFon the usage of non-standard signatures.An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
Quoted 5 lines
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
Provide a fix by suggesting users with this signature alternative and replacing.
This doesn't apply to either today's linux or -next.
It is dependent on a patch that is only in Andrew Morton's mmots. Your patch subject should show that in the brackets. Something like:
[PATCH -mmots] checkpatch: etc...
Adding --fix options to the obvious spelling typos in these signature by-lines tags are a different case but I think this is a relatively low value patch as I don't believe these tags should be limited in this way.
Maybe Andrew likes it.
scripts/checkpatch.pl
Quoted 1 line
scripts/checkpatch.pl +0
-0
[]
Quoted 8 lines
scripts/checkpatch.pl +0
-0@@ -2832,6 +2832,9 @@ sub process {if ($sign_off !~ /$signature_tags/) {
my $suggested_signature =find_standard_signature($sign_off);scripts/checkpatch.pl +3
-0+ if ($sign_off =~ /co-authored-by:/i) { + $suggested_signature = "Co-developed-by:"; + }
PATCH 1/1 checkpatch: add fix for non-standard signature - co-authored-by
Currently, checkpatch.pl warns us for BAD_SIGN_OFF on the usage of non-standard signatures.
An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
Provide a fix by suggesting users with this signature alternative and replacing.
--- scripts/checkpatch.pl | 3 +++ 1 file changed, 3 insertions(+)
scripts/checkpatch.pl +3 -0
@@ -2832,6 +2832,9 @@ sub process { if ($sign_off !~ /$signature_tags/) { my $suggested_signature = find_standard_signature($sign_off); + if ($sign_off =~ /co-authored-by:/i) { + $suggested_signature = "Co-developed-by:"; + } if ($suggested_signature eq "") { WARN("BAD_SIGN_OFF", "Non-standard signature: $sign_off\n" . $herecurr);
Signature · 6 lines
2.17.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Joe Perches
maintainerRe: [PATCH -mmots] checkpatch: add fix for non-standard signature - co-authored-by
Quoted 9 lines · Aditya Srivastava
Currently, checkpatch.pl warns us for
BAD_SIGN_OFFon the usage of non-standard signatures.An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
I'm not going to ack this as I don't mind non-standard signatures.
You should also always use patch subject versioning and include a changelog after the --- line.
Quoted 23 lines
Provide a fix by suggesting users with this signature alternative and replacing.
Signed-off-by Aditya Srivastava <[email protected]>---
scripts/checkpatch.pl | 3 +++1 file changed, 3 insertions(+)scripts/checkpatch.pl +0
-0@@ -2832,6 +2832,9 @@ sub process {if ($sign_off !~ /$signature_tags/) {
my $suggested_signature =find_standard_signature($sign_off);scripts/checkpatch.pl +3
-0+ if ($sign_off =~ /co-authored-by:/i) { + $suggested_signature = "Co-developed-by:"; + }if ($suggested_signature eq "") { WARN("BAD_SIGN_OFF",
"Non-standard signature: $sign_off\n" . $herecurr);
Re: [PATCH -mmots] checkpatch: add fix for non-standard signature - co-authored-by
Quoted 4 lines · Joe Perches
Quoted 3 lines · Aditya Srivastava
Currently, checkpatch.pl warns us for
BAD_SIGN_OFFon the usage of non-standard signatures.
Quoted 9 lines
Quoted 6 lines
An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
I'm not going to ack this as I don't mind non-standard signatures.
What do you suggest?
Should I drop this patch and move on?
Quoted 3 lines
You should also always use patch subject versioning and include a changelog after the --- line.
Ok, will do. Actually I had not included the version changelogs and versioning from our previous patch, as the subject for the patch had changed. Will keep in mind for future though.
Thanks
Aditya
Quoted 15 lines
Quoted 15 lines
Provide a fix by suggesting users with this signature alternative and replacing.
Signed-off-by Aditya Srivastava <[email protected]>---
scripts/checkpatch.pl | 3 +++1 file changed, 3 insertions(+)scripts/checkpatch.pl +0
-0@@ -2832,6 +2832,9 @@ sub process {
Quoted 10 lines
Quoted 8 lines
if ($sign_off !~ /$signature_tags/) {
my $suggested_signature =find_standard_signature($sign_off);scripts/checkpatch.pl +3
-0+ if ($sign_off =~ /co-authored-by:/i) { + $suggested_signature = "Co-developed-by:"; + }if ($suggested_signature eq "") { WARN("BAD_SIGN_OFF",
"Non-standard signature: $sign_off\n" . $herecurr);
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Re: [PATCH -mmots] checkpatch: add fix for non-standard signature - co-authored-by
Quoted 10 lines · Aditya
Quoted 8 lines · Joe Perches
Quoted 7 lines · Aditya Srivastava
Currently, checkpatch.pl warns us for
BAD_SIGN_OFFon the usage of non-standard signatures.An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
I'm not going to ack this as I don't mind non-standard signatures.
What do you suggest?
Should I drop this patch and move on?
Joe does not ack this, but he also does not nack it.
You either move on (which is perfectly fine), or
you either wait that Andrew Morton reviews it and accepts it because he thinks it useful, or
you reach out to the git committers that have been using Co-authored-by in the past and ask them if this kind of feature would have been helpful for them and you get an ack from them that convinces Andrew Morton to pick this.
I hope this helps. If nobody thinks it is a useful feature, move on to work others acknowledge as useful. We have enough stuff to work on.
Lukas
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Re: [PATCH -mmots] checkpatch: add fix for non-standard signature - co-authored-by
Quoted 28 lines · Lukas Bulwahn
Quoted 10 lines · Aditya
Quoted 8 lines · Joe Perches
Quoted 7 lines · Aditya Srivastava
Currently, checkpatch.pl warns us for
BAD_SIGN_OFFon the usage of non-standard signatures.An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
I'm not going to ack this as I don't mind non-standard signatures.
What do you suggest?
Should I drop this patch and move on?Joe does not ack this, but he also does not nack it.
You either move on (which is perfectly fine), or
you either wait that Andrew Morton reviews it and accepts it because he thinks it useful, or
Quoted 8 lines
you reach out to the git committers that have been using Co-authored-by in the past and ask them if this kind of feature would have been helpful for them and you get an ack from them that convinces Andrew Morton to pick this.
I hope this helps. If nobody thinks it is a useful feature, move on to work others acknowledge as useful. We have enough stuff to work on.
Okay. I get it now. Thanks a lot Lukas :)
--
Aditya
Quoted 2 lines
Lukas
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Re: [Linux-kernel-mentees] [PATCH -mmots] checkpatch: add fix for non-standard signature - co-authored-by
In reply to Lukas BulwahnQuoted 25 lines · Lukas Bulwahn
Quoted 10 lines · Aditya
Quoted 8 lines · Joe Perches
Quoted 7 lines · Aditya Srivastava
Currently, checkpatch.pl warns us for
BAD_SIGN_OFFon the usage of non-standard signatures.An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
I'm not going to ack this as I don't mind non-standard signatures.
What do you suggest?
Should I drop this patch and move on?Joe does not ack this, but he also does not nack it.
You either move on (which is perfectly fine), or
Quoted 7 lines
you either wait that Andrew Morton reviews it and accepts it because he thinks it useful, or
you reach out to the git committers that have been using Co-authored-by in the past and ask them if this kind of feature would have been helpful for them and you get an ack from them that convinces Andrew Morton to pick this.
co-developed-by is the correct tag for this. It is documented exactly for this reason, please do not try to use something that is not already accepted by the kernel developers for this type of thing.
thanks,
greg k-h
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Re: [Linux-kernel-mentees] [PATCH -mmots] checkpatch: add fix for non-standard signature - co-authored-by
Quoted 30 lines · Greg KH
Quoted 28 lines · Lukas Bulwahn
Quoted 10 lines · Aditya
Quoted 8 lines · Joe Perches
Quoted 7 lines · Aditya Srivastava
Currently, checkpatch.pl warns us for
BAD_SIGN_OFFon the usage of non-standard signatures.An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
I'm not going to ack this as I don't mind non-standard signatures.
What do you suggest?
Should I drop this patch and move on?Joe does not ack this, but he also does not nack it.
You either move on (which is perfectly fine), or
you either wait that Andrew Morton reviews it and accepts it because he thinks it useful, or
Quoted 9 lines
Quoted 4 lines
you reach out to the git committers that have been using Co-authored-by in the past and ask them if this kind of feature would have been helpful for them and you get an ack from them that convinces Andrew Morton to pick this.
co-developed-by is the correct tag for this. It is documented exactly for this reason, please do not try to use something that is not already accepted by the kernel developers for this type of thing.
Well, Greg, so do we get your Acked-by on a feature that checkpatch warns that Co-authored-by is non-standard and that then fixes up the patch automatically to Co-developed-by with checkpatch --fix?
If so, please add your Acked-by on this patch here and let Andrew know to pick it...
Lukas
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Re: [Linux-kernel-mentees] [PATCH -mmots] checkpatch: add fix for non-standard signature - co-authored-by
Quoted 41 lines · Lukas Bulwahn
Quoted 30 lines · Greg KH
Quoted 28 lines · Lukas Bulwahn
Quoted 10 lines · Aditya
Quoted 8 lines · Joe Perches
Quoted 7 lines · Aditya Srivastava
Currently, checkpatch.pl warns us for
BAD_SIGN_OFFon the usage of non-standard signatures.An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
I'm not going to ack this as I don't mind non-standard signatures.
What do you suggest?
Should I drop this patch and move on?Joe does not ack this, but he also does not nack it.
You either move on (which is perfectly fine), or
you either wait that Andrew Morton reviews it and accepts it because he thinks it useful, or
Quoted 9 lines
Quoted 4 lines
you reach out to the git committers that have been using Co-authored-by in the past and ask them if this kind of feature would have been helpful for them and you get an ack from them that convinces Andrew Morton to pick this.
co-developed-by is the correct tag for this. It is documented exactly for this reason, please do not try to use something that is not already accepted by the kernel developers for this type of thing.
Quoted 6 lines
Well, Greg, so do we get your Acked-by on a feature that checkpatch warns that Co-authored-by is non-standard and that then fixes up the patch automatically to Co-developed-by with checkpatch --fix?
If so, please add your Acked-by on this patch here and let Andrew know to pick it...
I have not reviewed the patch, sorry, just was commenting on the fact that we do have a standard for this.
thanks,
greg k-h
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Joe Perches
maintainerRe: [Linux-kernel-mentees] [PATCH -mmots] checkpatch: add fix for non-standard signature - co-authored-by
Quoted 10 lines · Greg KH
Quoted 9 lines · Lukas Bulwahn
Quoted 8 lines · Greg KH
Quoted 6 lines · Lukas Bulwahn
Quoted 5 lines · Aditya
Quoted 3 lines · Joe Perches
Quoted 2 lines · Aditya Srivastava
Currently, checkpatch.pl warns us for
BAD_SIGN_OFFon the usage of non-standard signatures.
[]
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
co-developed-by is the correct tag for this. It is documented exactly for this reason, please do not try to use something that is not already accepted by the kernel developers for this type of thing.
Postel's law should apply here.
And there is a concurrent thread on the ksummit-discuss mailing list that applies:
I hope all this becomes moot eventually.
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Re: [Linux-kernel-mentees] [PATCH -mmots] checkpatch: add fix for non-standard signature - co-authored-by
In reply to Lukas BulwahnQuoted 25 lines · Lukas Bulwahn
Quoted 10 lines · Aditya
Quoted 8 lines · Joe Perches
Quoted 7 lines · Aditya Srivastava
Currently, checkpatch.pl warns us for
BAD_SIGN_OFFon the usage of non-standard signatures.An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
I'm not going to ack this as I don't mind non-standard signatures.
What do you suggest?
Should I drop this patch and move on?Joe does not ack this, but he also does not nack it.
You either move on (which is perfectly fine), or
Quoted 8 lines
you either wait that Andrew Morton reviews it and accepts it because he thinks it useful, or
you reach out to the git committers that have been using Co-authored-by in the past and ask them if this kind of feature would have been helpful for them and you get an ack from them that convinces Andrew Morton to pick this.
Hi Lukas
I have generated a list of users who used co-authored-by tag, ordered by the number of times they used it (over v4.13..v5.8) here:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/signature_tags/co_authors.txt
However I am a bit unsure about who among these is still active and can perhaps give us Acked-by.
Thanks
Aditya
Quoted 5 lines
I hope this helps. If nobody thinks it is a useful feature, move on to work others acknowledge as useful. We have enough stuff to work on.
Lukas
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v6 1/1 checkpatch: add fix for non-standard signature - co-authored-by
In reply to AdityaCurrently, checkpatch.pl warns us for BAD_SIGN_OFF on the usage of non-standard signatures.
An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
Provide a fix by suggesting users with this signature alternative and replacing.
---
applies perfectly on the latest next-20201204 branch
changes in v2: replace commit specific example with brief evaluation
changes in v3: provide rationale to users for every signature tag suggestion; modify commit message describing arrival to conclusion in a structured way
changes in v4: modify rationale for certain suggestions
changes in v5: remove the tag deletion suggestions, ie "Generated-by" and "Celebrated-by"; rebase on last accepted changes; modify commit message
changes in v6: reduce tag suggestions to only "Co-authored-by"; modify commit message accordingly; include complete version changelog
scripts/checkpatch.pl | 3 +++ 1 file changed, 3 insertions(+)
scripts/checkpatch.pl +3 -0
@@ -2832,6 +2832,9 @@ sub process { if ($sign_off !~ /$signature_tags/) { my $suggested_signature = find_standard_signature($sign_off); + if ($sign_off =~ /co-authored-by:/i) { + $suggested_signature = "Co-developed-by:"; + } if ($suggested_signature eq "") { WARN("BAD_SIGN_OFF", "Non-standard signature: $sign_off\n" . $herecurr);
Signature · 6 lines
2.17.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Joe Perches
maintainerRe: [PATCH v6] checkpatch: add fix for non-standard signature - co-authored-by
Quoted 7 lines · Aditya Srivastava
Currently, checkpatch.pl warns us for
BAD_SIGN_OFFon the usage of non-standard signatures.An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
Quoted 7 lines
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
Provide a fix by suggesting users with this signature alternative and replacing.
Signed-off-by Aditya Srivastava <[email protected]>
lkml is a very active dumping ground and most people do not scan through each patch sent to it.
If you want specific people to see this and potentially ack this you should add them to the cc list.
Re: [PATCH v6] checkpatch: add fix for non-standard signature - co-authored-by
In reply to Aditya SrivastavaQuoted 37 lines · Aditya Srivastava
Currently, checkpatch.pl warns us for
BAD_SIGN_OFFon the usage of non-standard signatures.An evaluation on v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 43 are due to the use of 'Co-authored-by' tag, which may seem correct, but is not standard.
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
Provide a fix by suggesting users with this signature alternative and replacing.
Signed-off-by Aditya Srivastava <[email protected]>---
applies perfectly on the latest next-20201204 branchchanges in v2: replace commit specific example with brief evaluation
changes in v3: provide rationale to users for every signature tag suggestion; modify commit message describing arrival to conclusion in a structured way
changes in v4: modify rationale for certain suggestions
changes in v5: remove the tag deletion suggestions, ie "Generated-by" and "Celebrated-by"; rebase on last accepted changes; modify commit message
changes in v6: reduce tag suggestions to only "Co-authored-by"; modify commit message accordingly; include complete version changelog
scripts/checkpatch.pl | 3 +++ 1 file changed, 3 insertions(+)scripts/checkpatch.pl +0
-0
scripts/checkpatch.pl sub process { · line 2840
Quoted 9 lines
scripts/checkpatch.pl +3
-0@@ -2832,6 +2832,9 @@ sub process { if ($sign_off !~ /$signature_tags/) { my $suggested_signature = find_standard_signature($sign_off); + if ($sign_off =~ /co-authored-by:/i) { + $suggested_signature = "Co-developed-by:"; + } if ($suggested_signature eq "") { WARN("BAD_SIGN_OFF", "Non-standard signature: $sign_off\n" . $herecurr);
Hi Daniel and Peter
Sorry to disturb you. Actually we were planning to introduce a fix for suggesting users to use "Co-developed-by" tag over "Co-authored-by" and I noticed that you have earlier used "Co-authored-by" tag.
We feel that users perhaps use this tag as they are unaware of its standard equivalent tag, "Co-developed-by"
Do you think that this fix will be beneficial for future users? If so, can you please add your Acked-by to the patch?
Thanks
Aditya
Joe Perches
maintainerRe: [PATCH v6] checkpatch: add fix for non-standard signature - co-authored-by
Quoted 3 lines · Aditya
Quoted 2 lines · Aditya Srivastava
Currently, checkpatch.pl warns us for
BAD_SIGN_OFFon the usage of non-standard signatures.
[]
Quoted 6 lines
Quoted 5 lines
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
Provide a fix by suggesting users with this signature alternative and replacing.
Quoted 6 lines
we were planning to introduce a fix for
suggesting users to use "Co-developed-by" tag over "Co-authored-by" and I noticed that you have earlier used "Co-authored-by" tag.We feel that users perhaps use this tag as they are unaware of its standard equivalent tag, "Co-developed-by"
As I do not particularly approve of this patch,
"we" does not include "me", nor is it I presume
the royal usage.
Please specify who the "we" is here.
Re: [PATCH v6] checkpatch: add fix for non-standard signature - co-authored-by
Quoted 15 lines · Joe Perches
Quoted 3 lines · Aditya
Quoted 2 lines · Aditya Srivastava
Currently, checkpatch.pl warns us for
BAD_SIGN_OFFon the usage of non-standard signatures.[]
Quoted 10 lines
Quoted 5 lines
The standard signature equivalent for 'Co-authored-by' is 'Co-developed-by'.
Provide a fix by suggesting users with this signature alternative and replacing.
we were planning to introduce a fix for
suggesting users to use "Co-developed-by" tag over "Co-authored-by" and I noticed that you have earlier used "Co-authored-by" tag.
Quoted 9 lines
Quoted 2 lines
We feel that users perhaps use this tag as they are unaware of its standard equivalent tag, "Co-developed-by"
As I do not particularly approve of this patch,
"we" does not include "me", nor is it I presume
the royal usage.Please specify who the "we" is here.
Hi Daniel and Peter
Sorry to disturb you. Actually we (me and Lukas) were planning to introduce a fix for suggesting users to use "Co-developed-by" tag over "Co-authored-by" and I noticed that you have earlier used "Co-authored-by" tag.
I feel that users perhaps use this tag as they are unaware of its standard equivalent tag, "Co-developed-by"
Do you think that this fix will be beneficial for future users? If so, can you please add your Acked-by to the patch?
Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees