All of lore.kernel.org
 help / color / mirror / Atom feed
* [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for ERROR:GERRIT_CHANGE_ID
@ 2020-10-30  8:23 Aditya Srivastava
  2020-10-30  9:37 ` Aditya
  2020-10-30 10:45 ` Lukas Bulwahn
  0 siblings, 2 replies; 5+ messages in thread
From: Aditya Srivastava @ 2020-10-30  8:23 UTC (permalink / raw)
  To: lukas.bulwahn; +Cc: linux-kernel-mentees, Aditya Srivastava

Currently, whenever gerrit change id is present in commit, checkpatch.pl
gives an error to remove the change id before submitting the patch.

For eg, running checkpatch on commit adc311a5bbf6 ("iwlwifi: bump FW
API to 53 for 22000 series"), causes error:

ERROR:GERRIT_CHANGE_ID: Remove Gerrit Change-Id's before submitting
upstream
Change-Id: I5725e46394f3f53c3069723fd513cc53c7df383d

But there is no --fix option present for this error message, in case
user wants to fix it immediately.

To fix this error, delete the corresponding line from the patchfile when
checkpatch.pl is executed with --fix

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
 scripts/checkpatch.pl | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7800a090e8fe..5329927fc1c1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2852,8 +2852,11 @@ sub process {
 
 # Check for Gerrit Change-Ids not in any patch context
 		if ($realfile eq '' && !$has_patch_separator && $line =~ /^\s*change-id:/i) {
-			ERROR("GERRIT_CHANGE_ID",
-			      "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr);
+			if (ERROR("GERRIT_CHANGE_ID",
+			          "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr) &&
+			    $fix) {
+                                fix_delete_line($fixlinenr, $rawline);
+                        }
 		}
 
 # Check if the commit log is in a possible stack dump
-- 
2.17.1

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for ERROR:GERRIT_CHANGE_ID
  2020-10-30  8:23 [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for ERROR:GERRIT_CHANGE_ID Aditya Srivastava
@ 2020-10-30  9:37 ` Aditya
  2020-10-30 10:49   ` Lukas Bulwahn
  2020-10-30 10:45 ` Lukas Bulwahn
  1 sibling, 1 reply; 5+ messages in thread
From: Aditya @ 2020-10-30  9:37 UTC (permalink / raw)
  To: lukas.bulwahn; +Cc: linux-kernel-mentees

On 30/10/20 1:53 pm, Aditya Srivastava wrote:
> Currently, whenever gerrit change id is present in commit, checkpatch.pl
> gives an error to remove the change id before submitting the patch.
> 
> For eg, running checkpatch on commit adc311a5bbf6 ("iwlwifi: bump FW
> API to 53 for 22000 series"), causes error:
> 
> ERROR:GERRIT_CHANGE_ID: Remove Gerrit Change-Id's before submitting
> upstream
> Change-Id: I5725e46394f3f53c3069723fd513cc53c7df383d
> 
> But there is no --fix option present for this error message, in case
> user wants to fix it immediately.
> 
> To fix this error, delete the corresponding line from the patchfile when
> checkpatch.pl is executed with --fix
> 
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
>  scripts/checkpatch.pl | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7800a090e8fe..5329927fc1c1 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2852,8 +2852,11 @@ sub process {
>  
>  # Check for Gerrit Change-Ids not in any patch context
>  		if ($realfile eq '' && !$has_patch_separator && $line =~ /^\s*change-id:/i) {
> -			ERROR("GERRIT_CHANGE_ID",
> -			      "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr);
> +			if (ERROR("GERRIT_CHANGE_ID",
> +			          "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr) &&
> +			    $fix) {
> +                                fix_delete_line($fixlinenr, $rawline);
> +                        }
>  		}
>  
>  # Check if the commit log is in a possible stack dump
> 

Sir I have a doubt: Can I start working on another patch and send that
too before the first one gets accepted?

Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for ERROR:GERRIT_CHANGE_ID
  2020-10-30  8:23 [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for ERROR:GERRIT_CHANGE_ID Aditya Srivastava
  2020-10-30  9:37 ` Aditya
@ 2020-10-30 10:45 ` Lukas Bulwahn
  1 sibling, 0 replies; 5+ messages in thread
From: Lukas Bulwahn @ 2020-10-30 10:45 UTC (permalink / raw)
  To: Aditya Srivastava; +Cc: linux-kernel-mentees


I would drop the ERROR:, just say: add fix option for GERRIT_CHANGE_ID

On Fri, 30 Oct 2020, Aditya Srivastava wrote:

> Currently, whenever gerrit change id is present in commit, checkpatch.pl
...whenever a Gerrir Change-Id is present in a commit,

> gives an error to remove the change id before submitting the patch.
>
s/gives an error/warns/
s/change id/Change-Id/
 
> For eg, running checkpatch on commit adc311a5bbf6 ("iwlwifi: bump FW

Just: "E.g.," not "For eg"
 
> API to 53 for 22000 series"), causes error:

s/causes error/reports this error/

> 
> ERROR:GERRIT_CHANGE_ID: Remove Gerrit Change-Id's before submitting
> upstream
> Change-Id: I5725e46394f3f53c3069723fd513cc53c7df383d
>

The default setup does not show the type name, I would suggest to just 
show the default output from checkpatch.pl.
 
> But there is no --fix option present for this error message, in case
> user wants to fix it immediately.
> 
> To fix this error, delete the corresponding line from the patchfile when
> checkpatch.pl is executed with --fix
> 

Provide a simple fix option by simply deleting the indicated line.

> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
>  scripts/checkpatch.pl | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7800a090e8fe..5329927fc1c1 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2852,8 +2852,11 @@ sub process {
>  
>  # Check for Gerrit Change-Ids not in any patch context
>  		if ($realfile eq '' && !$has_patch_separator && $line =~ /^\s*change-id:/i) {
> -			ERROR("GERRIT_CHANGE_ID",
> -			      "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr);
> +			if (ERROR("GERRIT_CHANGE_ID",
> +			          "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr) &&
> +			    $fix) {
> +                                fix_delete_line($fixlinenr, $rawline);
> +                        }

The change itself looks good to me.

Rework the commit message, send it out again here for a second check and 
if it is fine then you can submit it to Joe and lkml.
>  		}
>  
>  # Check if the commit log is in a possible stack dump
> -- 
> 2.17.1
> 
> 
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for ERROR:GERRIT_CHANGE_ID
  2020-10-30  9:37 ` Aditya
@ 2020-10-30 10:49   ` Lukas Bulwahn
  2020-10-30 10:54     ` Aditya
  0 siblings, 1 reply; 5+ messages in thread
From: Lukas Bulwahn @ 2020-10-30 10:49 UTC (permalink / raw)
  To: Aditya; +Cc: linux-kernel-mentees



On Fri, 30 Oct 2020, Aditya wrote:

> On 30/10/20 1:53 pm, Aditya Srivastava wrote:
> > Currently, whenever gerrit change id is present in commit, checkpatch.pl
> > gives an error to remove the change id before submitting the patch.
> > 
> > For eg, running checkpatch on commit adc311a5bbf6 ("iwlwifi: bump FW
> > API to 53 for 22000 series"), causes error:
> > 
> > ERROR:GERRIT_CHANGE_ID: Remove Gerrit Change-Id's before submitting
> > upstream
> > Change-Id: I5725e46394f3f53c3069723fd513cc53c7df383d
> > 
> > But there is no --fix option present for this error message, in case
> > user wants to fix it immediately.
> > 
> > To fix this error, delete the corresponding line from the patchfile when
> > checkpatch.pl is executed with --fix
> > 
> > Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> > ---
> >  scripts/checkpatch.pl | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 7800a090e8fe..5329927fc1c1 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -2852,8 +2852,11 @@ sub process {
> >  
> >  # Check for Gerrit Change-Ids not in any patch context
> >  		if ($realfile eq '' && !$has_patch_separator && $line =~ /^\s*change-id:/i) {
> > -			ERROR("GERRIT_CHANGE_ID",
> > -			      "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr);
> > +			if (ERROR("GERRIT_CHANGE_ID",
> > +			          "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr) &&
> > +			    $fix) {
> > +                                fix_delete_line($fixlinenr, $rawline);
> > +                        }
> >  		}
> >  
> >  # Check if the commit log is in a possible stack dump
> > 
> 
> Sir I have a doubt: Can I start working on another patch and send that
> too before the first one gets accepted?
>

Sure, you can parallelize your work as much as you can.
Review takes some time: we all have other full-time jobs and many other 
tasks.

When you get feedback, though, I suggest to take that feedback immediately 
and consider reworking your patch.

Lukas
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for ERROR:GERRIT_CHANGE_ID
  2020-10-30 10:49   ` Lukas Bulwahn
@ 2020-10-30 10:54     ` Aditya
  0 siblings, 0 replies; 5+ messages in thread
From: Aditya @ 2020-10-30 10:54 UTC (permalink / raw)
  To: Lukas Bulwahn; +Cc: linux-kernel-mentees

On 30/10/20 4:19 pm, Lukas Bulwahn wrote:
> 
> 
> On Fri, 30 Oct 2020, Aditya wrote:
> 
>> On 30/10/20 1:53 pm, Aditya Srivastava wrote:
>>> Currently, whenever gerrit change id is present in commit, checkpatch.pl
>>> gives an error to remove the change id before submitting the patch.
>>>
>>> For eg, running checkpatch on commit adc311a5bbf6 ("iwlwifi: bump FW
>>> API to 53 for 22000 series"), causes error:
>>>
>>> ERROR:GERRIT_CHANGE_ID: Remove Gerrit Change-Id's before submitting
>>> upstream
>>> Change-Id: I5725e46394f3f53c3069723fd513cc53c7df383d
>>>
>>> But there is no --fix option present for this error message, in case
>>> user wants to fix it immediately.
>>>
>>> To fix this error, delete the corresponding line from the patchfile when
>>> checkpatch.pl is executed with --fix
>>>
>>> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
>>> ---
>>>  scripts/checkpatch.pl | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index 7800a090e8fe..5329927fc1c1 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -2852,8 +2852,11 @@ sub process {
>>>  
>>>  # Check for Gerrit Change-Ids not in any patch context
>>>  		if ($realfile eq '' && !$has_patch_separator && $line =~ /^\s*change-id:/i) {
>>> -			ERROR("GERRIT_CHANGE_ID",
>>> -			      "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr);
>>> +			if (ERROR("GERRIT_CHANGE_ID",
>>> +			          "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr) &&
>>> +			    $fix) {
>>> +                                fix_delete_line($fixlinenr, $rawline);
>>> +                        }
>>>  		}
>>>  
>>>  # Check if the commit log is in a possible stack dump
>>>
>>
>> Sir I have a doubt: Can I start working on another patch and send that
>> too before the first one gets accepted?
>>
> 
> Sure, you can parallelize your work as much as you can.
> Review takes some time: we all have other full-time jobs and many other 
> tasks.
> 
> When you get feedback, though, I suggest to take that feedback immediately 
> and consider reworking your patch.
> 
> Lukas
> 

Thanks Sir. Sure, will keep that in mind.

Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-10-30 10:54 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-30  8:23 [Linux-kernel-mentees] [PATCH] checkpatch: add fix option for ERROR:GERRIT_CHANGE_ID Aditya Srivastava
2020-10-30  9:37 ` Aditya
2020-10-30 10:49   ` Lukas Bulwahn
2020-10-30 10:54     ` Aditya
2020-10-30 10:45 ` Lukas Bulwahn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.