Subject: [PATCH 0/2] Resolve Co-Developed-by issues
Recently tag Co-Developed-by was added (via a patch to
Documention/process/5.Posting.rst).
see https://lkml.org/lkml/2017/11/16/257
There has already been some discontent expressed over this tag.
A recent patch using the tag generated this response from akmp (on LKML)
I prefer not to include tags which aren't listed in
Documentation/process/submitting-patches.rst, but I now see that
some bright spark added Co-Developed-by: to
Documentation/process/5.Posting.rst, so the two files are a)
duplicative and b) out of sync
Also, there has been comment over the use of the second capitalized
character 'D'.
This patch set attempts to either resolve this issue or bury it for
good.
May the bike shedding begin :)
Joe Perches (1):
checkpatch: add check for tag Co-Developed-by
Tobin C. Harding (1):
docs: add Co-Developed-by docs
Documentation/process/submitting-patches.rst | 9 ++++-
scripts/checkpatch.pl | 58 +++++++++++++++++-----------
2 files changed, 42 insertions(+), 25 deletions(-)
--
2.7.4
When Co-Developed-by tag was added, docs were only added to
Documention/process/5.Posting.rst and were not added to
Documention/process/submitting-patches.rst
Add documentation to Documention/process/submitting-patches.rst
Signed-off-by: Tobin C. Harding <[email protected]>
---
The text in this patch is copied basically verbatim from
Documentation/process/submitting-patches.rst
Documentation/process/submitting-patches.rst | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index 1ef19d3a3eee..698360641ecd 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -510,8 +510,8 @@ tracking your trees, and to people trying to troubleshoot bugs in your
tree.
-12) When to use Acked-by: and Cc:
----------------------------------
+12) When to use Acked-by: and Cc:, and Co-Developed-by:
+-------------------------------------------------------
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.
@@ -543,6 +543,11 @@ person it names - but it should indicate that this person was copied on the
patch. This tag documents that potentially interested parties
have been included in the discussion.
+A Co-Developed-by: states that the patch was also created by another developer
+along with the original author. This is useful at times when multiple people
+work on a single patch. Note, this person also needs to have a Signed-off-by:
+line in the patch as well.
+
13) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:
--------------------------------------------------------------------------
--
2.7.4
From: Joe Perches <[email protected]>
Recently signature tag Co-Developed-by was added to the
kernel (Documentation/process/5.Posting.rst). checkpatch.pl doesn't know
about it yet. All prior tags used all lowercase characters except for first
character. Checks for this format had to be re-worked to allow for the
new tag.
Cc: Greg Kroah-Hartman <[email protected]>
Reviewed-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Tobin C. Harding <[email protected]>
---
scripts/checkpatch.pl | 58 +++++++++++++++++++++++++++++++--------------------
1 file changed, 35 insertions(+), 23 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3d4040322ae1..fbe2ae2d035f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -461,16 +461,18 @@ our $logFunctions = qr{(?x:
seq_vprintf|seq_printf|seq_puts
)};
-our $signature_tags = qr{(?xi:
- Signed-off-by:|
- Acked-by:|
- Tested-by:|
- Reviewed-by:|
- Reported-by:|
- Suggested-by:|
- To:|
- Cc:
-)};
+our @valid_signatures = (
+ "Signed-off-by:",
+ "Acked-by:",
+ "Tested-by:",
+ "Reviewed-by:",
+ "Reported-by:",
+ "Suggested-by:",
+ "Co-Developed-by:",
+ "To:",
+ "Cc:"
+);
+my $signature_tags = "(?x:" . join('|', @valid_signatures) . ")";
our @typeListMisordered = (
qr{char\s+(?:un)?signed},
@@ -2193,6 +2195,17 @@ sub pos_last_openparen {
return length(expand_tabs(substr($line, 0, $last_openparen))) + 1;
}
+sub get_preferred_sign_off {
+ my ($sign_off) = @_;
+
+ foreach my $sig (@valid_signatures) {
+ if (lc($sign_off) eq lc($sig)) {
+ return $sig;
+ }
+ }
+ return "";
+}
+
sub process {
my $filename = shift;
@@ -2499,35 +2512,34 @@ sub process {
my $sign_off = $2;
my $space_after = $3;
my $email = $4;
- my $ucfirst_sign_off = ucfirst(lc($sign_off));
+ my $preferred_sign_off = ucfirst(lc($sign_off));
- if ($sign_off !~ /$signature_tags/) {
+ if ($sign_off !~ /$signature_tags/i) {
WARN("BAD_SIGN_OFF",
"Non-standard signature: $sign_off\n" . $herecurr);
- }
- if (defined $space_before && $space_before ne "") {
+ } elsif ($sign_off !~ /$signature_tags/) {
+ $preferred_sign_off = get_preferred_sign_off($sign_off);
if (WARN("BAD_SIGN_OFF",
- "Do not use whitespace before $ucfirst_sign_off\n" . $herecurr) &&
+ "'$preferred_sign_off' is the preferred signature form\n" . $herecurr) &&
$fix) {
- $fixed[$fixlinenr] =
- "$ucfirst_sign_off $email";
+ $fixed[$fixlinenr] = "$preferred_sign_off $email";
}
}
- if ($sign_off =~ /-by:$/i && $sign_off ne $ucfirst_sign_off) {
+ if (defined $space_before && $space_before ne "") {
if (WARN("BAD_SIGN_OFF",
- "'$ucfirst_sign_off' is the preferred signature form\n" . $herecurr) &&
+ "Do not use whitespace before $preferred_sign_off\n" . $herecurr) &&
$fix) {
$fixed[$fixlinenr] =
- "$ucfirst_sign_off $email";
+ "$preferred_sign_off $email";
}
-
}
+
if (!defined $space_after || $space_after ne " ") {
if (WARN("BAD_SIGN_OFF",
- "Use a single space after $ucfirst_sign_off\n" . $herecurr) &&
+ "Use a single space after $preferred_sign_off\n" . $herecurr) &&
$fix) {
$fixed[$fixlinenr] =
- "$ucfirst_sign_off $email";
+ "$preferred_sign_off $email";
}
}
--
2.7.4
On Mon, Mar 05, 2018 at 02:58:21PM +1100, Tobin C. Harding wrote:
> -12) When to use Acked-by: and Cc:
> ----------------------------------
> +12) When to use Acked-by: and Cc:, and Co-Developed-by:
> +-------------------------------------------------------
+12) When to use Acked-by:, Cc:, and Co-Developed-by:
On Mon, 2018-03-05 at 14:58 +1100, Tobin C. Harding wrote:
> From: Joe Perches <[email protected]>
I still think this "Co-Developed-by" stuff is unnecessary.
> Recently signature tag Co-Developed-by was added to the
> kernel (Documentation/process/5.Posting.rst). checkpatch.pl doesn't know
> about it yet. All prior tags used all lowercase characters except for first
> character. Checks for this format had to be re-worked to allow for the
> new tag.
>
> Cc: Greg Kroah-Hartman <[email protected]>
>
> Reviewed-by: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Tobin C. Harding <[email protected]>
> ---
> scripts/checkpatch.pl | 58 +++++++++++++++++++++++++++++++--------------------
> 1 file changed, 35 insertions(+), 23 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3d4040322ae1..fbe2ae2d035f 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -461,16 +461,18 @@ our $logFunctions = qr{(?x:
> seq_vprintf|seq_printf|seq_puts
> )};
>
> -our $signature_tags = qr{(?xi:
> - Signed-off-by:|
> - Acked-by:|
> - Tested-by:|
> - Reviewed-by:|
> - Reported-by:|
> - Suggested-by:|
> - To:|
> - Cc:
> -)};
> +our @valid_signatures = (
> + "Signed-off-by:",
> + "Acked-by:",
> + "Tested-by:",
> + "Reviewed-by:",
> + "Reported-by:",
> + "Suggested-by:",
> + "Co-Developed-by:",
> + "To:",
> + "Cc:"
> +);
> +my $signature_tags = "(?x:" . join('|', @valid_signatures) . ")";
>
> our @typeListMisordered = (
> qr{char\s+(?:un)?signed},
> @@ -2193,6 +2195,17 @@ sub pos_last_openparen {
> return length(expand_tabs(substr($line, 0, $last_openparen))) + 1;
> }
>
> +sub get_preferred_sign_off {
> + my ($sign_off) = @_;
> +
> + foreach my $sig (@valid_signatures) {
> + if (lc($sign_off) eq lc($sig)) {
> + return $sig;
> + }
> + }
> + return "";
> +}
> +
> sub process {
> my $filename = shift;
>
> @@ -2499,35 +2512,34 @@ sub process {
> my $sign_off = $2;
> my $space_after = $3;
> my $email = $4;
> - my $ucfirst_sign_off = ucfirst(lc($sign_off));
> + my $preferred_sign_off = ucfirst(lc($sign_off));
>
> - if ($sign_off !~ /$signature_tags/) {
> + if ($sign_off !~ /$signature_tags/i) {
> WARN("BAD_SIGN_OFF",
> "Non-standard signature: $sign_off\n" . $herecurr);
> - }
> - if (defined $space_before && $space_before ne "") {
> + } elsif ($sign_off !~ /$signature_tags/) {
> + $preferred_sign_off = get_preferred_sign_off($sign_off);
> if (WARN("BAD_SIGN_OFF",
> - "Do not use whitespace before $ucfirst_sign_off\n" . $herecurr) &&
> + "'$preferred_sign_off' is the preferred signature form\n" . $herecurr) &&
> $fix) {
> - $fixed[$fixlinenr] =
> - "$ucfirst_sign_off $email";
> + $fixed[$fixlinenr] = "$preferred_sign_off $email";
> }
> }
> - if ($sign_off =~ /-by:$/i && $sign_off ne $ucfirst_sign_off) {
> + if (defined $space_before && $space_before ne "") {
> if (WARN("BAD_SIGN_OFF",
> - "'$ucfirst_sign_off' is the preferred signature form\n" . $herecurr) &&
> + "Do not use whitespace before $preferred_sign_off\n" . $herecurr) &&
> $fix) {
> $fixed[$fixlinenr] =
> - "$ucfirst_sign_off $email";
> + "$preferred_sign_off $email";
> }
> -
> }
> +
> if (!defined $space_after || $space_after ne " ") {
> if (WARN("BAD_SIGN_OFF",
> - "Use a single space after $ucfirst_sign_off\n" . $herecurr) &&
> + "Use a single space after $preferred_sign_off\n" . $herecurr) &&
> $fix) {
> $fixed[$fixlinenr] =
> - "$ucfirst_sign_off $email";
> + "$preferred_sign_off $email";
> }
> }
>
On Mon, Mar 05, 2018 at 04:11:35AM -0800, Matthew Wilcox wrote:
> On Mon, Mar 05, 2018 at 02:58:21PM +1100, Tobin C. Harding wrote:
> > -12) When to use Acked-by: and Cc:
> > ----------------------------------
> > +12) When to use Acked-by: and Cc:, and Co-Developed-by:
> > +-------------------------------------------------------
>
> +12) When to use Acked-by:, Cc:, and Co-Developed-by:
thanks, sloppy work by me :)
Tobin
On Mon, 5 Mar 2018 14:58:21 +1100
"Tobin C. Harding" <[email protected]> wrote:
> When Co-Developed-by tag was added, docs were only added to
> Documention/process/5.Posting.rst and were not added to
> Documention/process/submitting-patches.rst
>
> Add documentation to Documention/process/submitting-patches.rst
Someday we should really pull those documents together into a coherent
whole so we don't have these weird synchronization issues. But it's much
easier to just apply this, so that's what I've done for now, with Willy's
comma tweak.
I'll leave the checkpatch part to Joe.
Thanks,
jon
On Wed, Mar 07, 2018 at 10:16:14AM -0700, Jonathan Corbet wrote:
> On Mon, 5 Mar 2018 14:58:21 +1100
> "Tobin C. Harding" <[email protected]> wrote:
>
> > When Co-Developed-by tag was added, docs were only added to
> > Documention/process/5.Posting.rst and were not added to
> > Documention/process/submitting-patches.rst
> >
> > Add documentation to Documention/process/submitting-patches.rst
>
> Someday we should really pull those documents together into a coherent
> whole so we don't have these weird synchronization issues. But it's much
> easier to just apply this, so that's what I've done for now, with Willy's
> comma tweak.
In case you still have my patch to Documentation/process/5.Posting.rst
pending, we'll have a mismatch between Co-developed-by there (which has been
used a number of times in the past), and Co-Developed-by in
submitting-patches.rst and checkpatch and Co-developed-by (which was used
exactly once in Linus' tree). That should be coherent at least...
Thanks,
Dominik