Re: [PATCH] checkpatch: add check for tag Co-Developed-by

[Tobin C. Harding]

On Thu, Dec 07, 2017 at 08:26:58AM -0800, Joe Perches wrote:
> On Thu, 2017-12-07 at 11:59 +1100, Tobin C. Harding wrote:
> > 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.
> > 
> > Add checkpatch checks for Co-Developed-by tag.
> 
> This patch is not extensible.
> 
> If this is to be done at all, the Co-Developed-by:
> should not be a special case because a new and
> different one might be added in the future.

I agree, fixing individual corner cases is bad style.

> Better to verify that a case insensitive match exists
> in $signature_tags and then complain if the match is
> not exact.

I'll have a go at this.

thanks,
Tobin.

Re: [PATCH] checkpatch: add check for tag Co-Developed-by

[Tobin C. Harding]

On Thu, Dec 07, 2017 at 08:26:58AM -0800, Joe Perches wrote:
> On Thu, 2017-12-07 at 11:59 +1100, Tobin C. Harding wrote:
> > 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.
> > 
> > Add checkpatch checks for Co-Developed-by tag.
> 
> This patch is not extensible.
> 
> If this is to be done at all, the Co-Developed-by:
> should not be a special case because a new and
> different one might be added in the future.

I agree, fixing individual corner cases is bad style.

> Better to verify that a case insensitive match exists
> in $signature_tags and then complain if the match is
> not exact.

I'll have a go at this.

thanks,
Tobin.

Re: [PATCH] checkpatch: add check for tag Co-Developed-by

[Joe Perches]

On Thu, 2017-12-07 at 11:59 +1100, Tobin C. Harding wrote:
> 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.
> 
> Add checkpatch checks for Co-Developed-by tag.

This patch is not extensible.

If this is to be done at all, the Co-Developed-by:
should not be a special case because a new and
different one might be added in the future.

Better to verify that a case insensitive match exists
in $signature_tags and then complain if the match is
not exact.

> Cc: Greg Kroah-Hartman 
> Signed-off-by: Tobin C. Harding 
> ---
>  scripts/checkpatch.pl | 22 --
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 040aa79e1d9d..a7d2cdcec6a6 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -468,6 +468,7 @@ our $signature_tags = qr{(?xi:
>   Reviewed-by:|
>   Reported-by:|
>   Suggested-by:|
> + Co-Developed-by:|
>   To:|
>   Cc:
>  )};
> @@ -2468,6 +2469,8 @@ sub process {
>   my $space_after = $3;
>   my $email = $4;
>   my $ucfirst_sign_off = ucfirst(lc($sign_off));
> + my $preferred_signature = "";
> + my $co_dev_by_tag = 'Co-Developed-by:';
>  
>   if ($sign_off !~ /$signature_tags/) {
>   WARN("BAD_SIGN_OFF",
> @@ -2481,15 +2484,22 @@ sub process {
>   "$ucfirst_sign_off $email";
>   }
>   }
> - if ($sign_off =~ /-by:$/i && $sign_off ne 
> $ucfirst_sign_off) {
> - if (WARN("BAD_SIGN_OFF",
> -  "'$ucfirst_sign_off' is the preferred 
> signature form\n" . $herecurr) &&
> - $fix) {
> - $fixed[$fixlinenr] =
> - "$ucfirst_sign_off $email";
> + if ($sign_off =~ /$co_dev_by_tag$/i) {
> + if ($sign_off ne $co_dev_by_tag) {
> + $preferred_signature = $co_dev_by_tag;
>   }
> + } elsif ($sign_off =~ /-by:$/i && $sign_off ne 
> $ucfirst_sign_off) {
> + $preferred_signature = $ucfirst_sign_off;
>  
>   }
> +
> + if ($preferred_signature ne "") {
> + my $prefix = "'$preferred_signature' is the 
> preferred signature form\n";
> + if (WARN("BAD_SIGN_OFF", $prefix . $herecurr) 
> && $fix) {
> + $fixed[$fixlinenr] = "$ucfirst_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) &&

Re: [PATCH] checkpatch: add check for tag Co-Developed-by

[Joe Perches]

On Thu, 2017-12-07 at 11:59 +1100, Tobin C. Harding wrote:
> 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.
> 
> Add checkpatch checks for Co-Developed-by tag.

This patch is not extensible.

If this is to be done at all, the Co-Developed-by:
should not be a special case because a new and
different one might be added in the future.

Better to verify that a case insensitive match exists
in $signature_tags and then complain if the match is
not exact.

> Cc: Greg Kroah-Hartman 
> Signed-off-by: Tobin C. Harding 
> ---
>  scripts/checkpatch.pl | 22 --
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 040aa79e1d9d..a7d2cdcec6a6 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -468,6 +468,7 @@ our $signature_tags = qr{(?xi:
>   Reviewed-by:|
>   Reported-by:|
>   Suggested-by:|
> + Co-Developed-by:|
>   To:|
>   Cc:
>  )};
> @@ -2468,6 +2469,8 @@ sub process {
>   my $space_after = $3;
>   my $email = $4;
>   my $ucfirst_sign_off = ucfirst(lc($sign_off));
> + my $preferred_signature = "";
> + my $co_dev_by_tag = 'Co-Developed-by:';
>  
>   if ($sign_off !~ /$signature_tags/) {
>   WARN("BAD_SIGN_OFF",
> @@ -2481,15 +2484,22 @@ sub process {
>   "$ucfirst_sign_off $email";
>   }
>   }
> - if ($sign_off =~ /-by:$/i && $sign_off ne 
> $ucfirst_sign_off) {
> - if (WARN("BAD_SIGN_OFF",
> -  "'$ucfirst_sign_off' is the preferred 
> signature form\n" . $herecurr) &&
> - $fix) {
> - $fixed[$fixlinenr] =
> - "$ucfirst_sign_off $email";
> + if ($sign_off =~ /$co_dev_by_tag$/i) {
> + if ($sign_off ne $co_dev_by_tag) {
> + $preferred_signature = $co_dev_by_tag;
>   }
> + } elsif ($sign_off =~ /-by:$/i && $sign_off ne 
> $ucfirst_sign_off) {
> + $preferred_signature = $ucfirst_sign_off;
>  
>   }
> +
> + if ($preferred_signature ne "") {
> + my $prefix = "'$preferred_signature' is the 
> preferred signature form\n";
> + if (WARN("BAD_SIGN_OFF", $prefix . $herecurr) 
> && $fix) {
> + $fixed[$fixlinenr] = "$ucfirst_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) &&

Re: [PATCH] checkpatch: add check for tag Co-Developed-by

[Joe Perches]

On Thu, 2017-12-07 at 07:55 +0100, Greg Kroah-Hartman wrote:
> On Wed, Dec 06, 2017 at 10:34:26PM -0800, Joe Perches wrote:
> > On Thu, 2017-12-07 at 11:59 +1100, Tobin C. Harding wrote:
> > > 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.
> > > 
> > > Add checkpatch checks for Co-Developed-by tag.
> > 
> > []
> > > +++ b/scripts/checkpatch.pl
> > > @@ -468,6 +468,7 @@ our $signature_tags = qr{(?xi:
> > >   Reviewed-by:|
> > >   Reported-by:|
> > >   Suggested-by:|
> > > + Co-Developed-by:|
> > 
> > I think the extra hyphenate form is poor at best.
> > Codeveloped-by: should be the only form.
> 
> That's not what Documentation/process/5.Posting.rst says, sorry.

So what? That's easy to change.

> So Tobin's patch is correct.

Not to me.

Re: [PATCH] checkpatch: add check for tag Co-Developed-by

[Joe Perches]

On Thu, 2017-12-07 at 07:55 +0100, Greg Kroah-Hartman wrote:
> On Wed, Dec 06, 2017 at 10:34:26PM -0800, Joe Perches wrote:
> > On Thu, 2017-12-07 at 11:59 +1100, Tobin C. Harding wrote:
> > > 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.
> > > 
> > > Add checkpatch checks for Co-Developed-by tag.
> > 
> > []
> > > +++ b/scripts/checkpatch.pl
> > > @@ -468,6 +468,7 @@ our $signature_tags = qr{(?xi:
> > >   Reviewed-by:|
> > >   Reported-by:|
> > >   Suggested-by:|
> > > + Co-Developed-by:|
> > 
> > I think the extra hyphenate form is poor at best.
> > Codeveloped-by: should be the only form.
> 
> That's not what Documentation/process/5.Posting.rst says, sorry.

So what? That's easy to change.

> So Tobin's patch is correct.

Not to me.

Re: [PATCH] checkpatch: add check for tag Co-Developed-by

[Greg Kroah-Hartman]

On Thu, Dec 07, 2017 at 11:59:36AM +1100, Tobin C. Harding wrote:
> 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.
> 
> Add checkpatch checks for Co-Developed-by tag.
> 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Tobin C. Harding 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH] checkpatch: add check for tag Co-Developed-by

[Greg Kroah-Hartman]

On Thu, Dec 07, 2017 at 11:59:36AM +1100, Tobin C. Harding wrote:
> 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.
> 
> Add checkpatch checks for Co-Developed-by tag.
> 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Tobin C. Harding 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH] checkpatch: add check for tag Co-Developed-by

[Greg Kroah-Hartman]

On Wed, Dec 06, 2017 at 10:34:26PM -0800, Joe Perches wrote:
> On Thu, 2017-12-07 at 11:59 +1100, Tobin C. Harding wrote:
> > 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.
> > 
> > Add checkpatch checks for Co-Developed-by tag.
> []
> > +++ b/scripts/checkpatch.pl
> > @@ -468,6 +468,7 @@ our $signature_tags = qr{(?xi:
> > Reviewed-by:|
> > Reported-by:|
> > Suggested-by:|
> > +   Co-Developed-by:|
> 
> I think the extra hyphenate form is poor at best.
> Codeveloped-by: should be the only form.

That's not what Documentation/process/5.Posting.rst says, sorry.

So Tobin's patch is correct.

thanks,

greg k-h

Re: [PATCH] checkpatch: add check for tag Co-Developed-by

[Greg Kroah-Hartman]

On Wed, Dec 06, 2017 at 10:34:26PM -0800, Joe Perches wrote:
> On Thu, 2017-12-07 at 11:59 +1100, Tobin C. Harding wrote:
> > 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.
> > 
> > Add checkpatch checks for Co-Developed-by tag.
> []
> > +++ b/scripts/checkpatch.pl
> > @@ -468,6 +468,7 @@ our $signature_tags = qr{(?xi:
> > Reviewed-by:|
> > Reported-by:|
> > Suggested-by:|
> > +   Co-Developed-by:|
> 
> I think the extra hyphenate form is poor at best.
> Codeveloped-by: should be the only form.

That's not what Documentation/process/5.Posting.rst says, sorry.

So Tobin's patch is correct.

thanks,

greg k-h

Re: [PATCH] checkpatch: add check for tag Co-Developed-by

[Joe Perches]

On Thu, 2017-12-07 at 11:59 +1100, Tobin C. Harding wrote:
> 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.
> 
> Add checkpatch checks for Co-Developed-by tag.
[]
> +++ b/scripts/checkpatch.pl
> @@ -468,6 +468,7 @@ our $signature_tags = qr{(?xi:
>   Reviewed-by:|
>   Reported-by:|
>   Suggested-by:|
> + Co-Developed-by:|

I think the extra hyphenate form is poor at best.
Codeveloped-by: should be the only form.

Re: [PATCH] checkpatch: add check for tag Co-Developed-by

[Joe Perches]

On Thu, 2017-12-07 at 11:59 +1100, Tobin C. Harding wrote:
> 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.
> 
> Add checkpatch checks for Co-Developed-by tag.
[]
> +++ b/scripts/checkpatch.pl
> @@ -468,6 +468,7 @@ our $signature_tags = qr{(?xi:
>   Reviewed-by:|
>   Reported-by:|
>   Suggested-by:|
> + Co-Developed-by:|

I think the extra hyphenate form is poor at best.
Codeveloped-by: should be the only form.