linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
       [not found] <20200810125354.xeijyh3v5upatrez@salamander>
@ 2020-08-17  9:43 ` Lukas Bulwahn
  2020-08-21 16:18 ` Ayush
  1 sibling, 0 replies; 16+ messages in thread
From: Lukas Bulwahn @ 2020-08-17  9:43 UTC (permalink / raw)
  To: Ayush; +Cc: linux-kernel-mentees


Dear Ayush,

The zeroth task is to learn suitable netiquette for the communication with 
the kernel community.

First, please do not top-post.

    A: Because we read from top to bottom, left to right.
    Q: Why should I start my reply below the quoted text?

    A: Because it messes up the order in which people normally read text.
    Q: Why is top-posting such a bad thing?

    A: The lost context.
    Q: What makes top-posted replies harder to read than bottom-posted?

    A: Yes.
    Q: Should I trim down the quoted part of an email to which I'm 
replying?

Second, please always CC: linux-kernel-mentees@lists.linuxfoundation.org.

Third, set up your email client according to the kernel community rules.



Then, the first task is to run checkpatch.pl on a few kernel patches and
collect the results. When you have that, please share your script with
me, e.g., in a github repository.


Hints to the first task:

Can you create a list of all non-merge commits that were added in the
version v5.8 of the kernel, i.e., all non-merge commits that are in v5.8
and not already in v5.7?

Can you share the script/command you executed and the resulting list on 
github?

Can you run your script on all commits of this list above and record
all checkpatch.pl reports, and store them in your github repository?

Can you suggest ideas how to aggregate the findings and create a
statistics? For example: Which type of error is reported most?
Can you implement that idea?


I also suggest to have a look at
the options ./scripts/checkpatch.pl --list-types and
./scripts/checkpatch.pl --show-types. The option --show-types changes
the output of checkpatch.pl to list type identifiers, so it is easier
to parse and aggregate the output.

Please also share the script you create for that purpose on your
github repository.



The second task is to pick one warning that appears often and improve
checkpatch.pl to handle that better and get it accepted by the kernel
community.

Hints to the second task follow when the first task is solved.

If you fail on any of those tasks, you are out of the selection process.


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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
       [not found] <20200810125354.xeijyh3v5upatrez@salamander>
  2020-08-17  9:43 ` [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl" Lukas Bulwahn
@ 2020-08-21 16:18 ` Ayush
  2020-08-22  8:06   ` Lukas Bulwahn
                     ` (2 more replies)
  1 sibling, 3 replies; 16+ messages in thread
From: Ayush @ 2020-08-21 16:18 UTC (permalink / raw)
  To: Lukas Bulwahn; +Cc: linux-kernel-mentees

August 17, 2020 3:13 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:

> Dear Ayush,
> 
> The zeroth task is to learn suitable netiquette for the communication with 
> the kernel community.
> 
> First, please do not top-post.
> 
> A: Because we read from top to bottom, left to right.
> Q: Why should I start my reply below the quoted text?
> 
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> 
> A: The lost context.
> Q: What makes top-posted replies harder to read than bottom-posted?
> 
> A: Yes.
> Q: Should I trim down the quoted part of an email to which I'm 
> replying?
> 
> Second, please always CC: linux-kernel-mentees@lists.linuxfoundation.org.
> 
> Third, set up your email client according to the kernel community rules.
> 
> Then, the first task is to run checkpatch.pl on a few kernel patches and
> collect the results. When you have that, please share your script with
> me, e.g., in a github repository.
> 
> Hints to the first task:
> 
> Can you create a list of all non-merge commits that were added in the
> version v5.8 of the kernel, i.e., all non-merge commits that are in v5.8
> and not already in v5.7?
> 
> Can you share the script/command you executed and the resulting list on 
> github?
> 
> Can you run your script on all commits of this list above and record
> all checkpatch.pl reports, and store them in your github repository?
> 
> Can you suggest ideas how to aggregate the findings and create a
> statistics? For example: Which type of error is reported most?
> Can you implement that idea?
> 
> I also suggest to have a look at
> the options ./scripts/checkpatch.pl --list-types and
> ./scripts/checkpatch.pl --show-types. The option --show-types changes
> the output of checkpatch.pl to list type identifiers, so it is easier
> to parse and aggregate the output.
> 
> Please also share the script you create for that purpose on your
> github repository.
> 
> The second task is to pick one warning that appears often and improve
> checkpatch.pl to handle that better and get it accepted by the kernel
> community.
> 
> Hints to the second task follow when the first task is solved.
> 
> If you fail on any of those tasks, you are out of the selection process.
> 
> Lukas

Sir,

I have attempted the task 1 and pushed the same to GitHub.

Please have a look and suggest improvements.

https://github.com/eldraco19/evalute_improve_checkpatch_pl

Please let me know if there are any issues with this.

Thanks,
Ayush Karn
_______________________________________________
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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-08-21 16:18 ` Ayush
@ 2020-08-22  8:06   ` Lukas Bulwahn
  2020-08-24 10:06   ` Ayush
  2020-08-26 13:51   ` Ayush
  2 siblings, 0 replies; 16+ messages in thread
From: Lukas Bulwahn @ 2020-08-22  8:06 UTC (permalink / raw)
  To: Ayush; +Cc: linux-kernel-mentees



On Fri, 21 Aug 2020, Ayush wrote:

> August 17, 2020 3:13 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
> 
> > Dear Ayush,
> > 
> > The zeroth task is to learn suitable netiquette for the communication with 
> > the kernel community.
> > 
> > First, please do not top-post.
> > 
> > A: Because we read from top to bottom, left to right.
> > Q: Why should I start my reply below the quoted text?
> > 
> > A: Because it messes up the order in which people normally read text.
> > Q: Why is top-posting such a bad thing?
> > 
> > A: The lost context.
> > Q: What makes top-posted replies harder to read than bottom-posted?
> > 
> > A: Yes.
> > Q: Should I trim down the quoted part of an email to which I'm 
> > replying?
> > 
> > Second, please always CC: linux-kernel-mentees@lists.linuxfoundation.org.
> > 
> > Third, set up your email client according to the kernel community rules.
> > 
> > Then, the first task is to run checkpatch.pl on a few kernel patches and
> > collect the results. When you have that, please share your script with
> > me, e.g., in a github repository.
> > 
> > Hints to the first task:
> > 
> > Can you create a list of all non-merge commits that were added in the
> > version v5.8 of the kernel, i.e., all non-merge commits that are in v5.8
> > and not already in v5.7?
> > 
> > Can you share the script/command you executed and the resulting list on 
> > github?
> > 
> > Can you run your script on all commits of this list above and record
> > all checkpatch.pl reports, and store them in your github repository?
> > 
> > Can you suggest ideas how to aggregate the findings and create a
> > statistics? For example: Which type of error is reported most?
> > Can you implement that idea?
> > 
> > I also suggest to have a look at
> > the options ./scripts/checkpatch.pl --list-types and
> > ./scripts/checkpatch.pl --show-types. The option --show-types changes
> > the output of checkpatch.pl to list type identifiers, so it is easier
> > to parse and aggregate the output.
> > 
> > Please also share the script you create for that purpose on your
> > github repository.
> > 
> > The second task is to pick one warning that appears often and improve
> > checkpatch.pl to handle that better and get it accepted by the kernel
> > community.
> > 
> > Hints to the second task follow when the first task is solved.
> > 
> > If you fail on any of those tasks, you are out of the selection process.
> > 
> > Lukas
> 
> Sir,
> 
> I have attempted the task 1 and pushed the same to GitHub.
> 
> Please have a look and suggest improvements.
> 
> https://github.com/eldraco19/evalute_improve_checkpatch_pl
> 
> Please let me know if there are any issues with this.
>

So far, so good.

Here are the questions we want to answer:

- So what are the 20 categories that occur most?

You are getting close to that answer, but you are not there yet.

Then look at the findings. For those 20 categories, are there specific 
findings that are multiple times false positives?

So, the script complains about something, but it does not get that the 
patch author wrote something completely unrelated to the error message.

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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-08-21 16:18 ` Ayush
  2020-08-22  8:06   ` Lukas Bulwahn
@ 2020-08-24 10:06   ` Ayush
  2020-08-27  5:28     ` Lukas Bulwahn
  2020-08-30 18:51     ` Ayush
  2020-08-26 13:51   ` Ayush
  2 siblings, 2 replies; 16+ messages in thread
From: Ayush @ 2020-08-24 10:06 UTC (permalink / raw)
  To: Lukas Bulwahn; +Cc: linux-kernel-mentees

August 22, 2020 1:36 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:

> On Fri, 21 Aug 2020, Ayush wrote:
> 
>> August 17, 2020 3:13 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
>> 
>> Dear Ayush,
>> 
>> The zeroth task is to learn suitable netiquette for the communication with
>> the kernel community.
>> 
>> First, please do not top-post.
>> 
>> A: Because we read from top to bottom, left to right.
>> Q: Why should I start my reply below the quoted text?
>> 
>> A: Because it messes up the order in which people normally read text.
>> Q: Why is top-posting such a bad thing?
>> 
>> A: The lost context.
>> Q: What makes top-posted replies harder to read than bottom-posted?
>> 
>> A: Yes.
>> Q: Should I trim down the quoted part of an email to which I'm
>> replying?
>> 
>> Second, please always CC: linux-kernel-mentees@lists.linuxfoundation.org.
>> 
>> Third, set up your email client according to the kernel community rules.
>> 
>> Then, the first task is to run checkpatch.pl on a few kernel patches and
>> collect the results. When you have that, please share your script with
>> me, e.g., in a github repository.
>> 
>> Hints to the first task:
>> 
>> Can you create a list of all non-merge commits that were added in the
>> version v5.8 of the kernel, i.e., all non-merge commits that are in v5.8
>> and not already in v5.7?
>> 
>> Can you share the script/command you executed and the resulting list on
>> github?
>> 
>> Can you run your script on all commits of this list above and record
>> all checkpatch.pl reports, and store them in your github repository?
>> 
>> Can you suggest ideas how to aggregate the findings and create a
>> statistics? For example: Which type of error is reported most?
>> Can you implement that idea?
>> 
>> I also suggest to have a look at
>> the options ./scripts/checkpatch.pl --list-types and
>> ./scripts/checkpatch.pl --show-types. The option --show-types changes
>> the output of checkpatch.pl to list type identifiers, so it is easier
>> to parse and aggregate the output.
>> 
>> Please also share the script you create for that purpose on your
>> github repository.
>> 
>> The second task is to pick one warning that appears often and improve
>> checkpatch.pl to handle that better and get it accepted by the kernel
>> community.
>> 
>> Hints to the second task follow when the first task is solved.
>> 
>> If you fail on any of those tasks, you are out of the selection process.
>> 
>> Lukas
>> 
>> Sir,
>> 
>> I have attempted the task 1 and pushed the same to GitHub.
>> 
>> Please have a look and suggest improvements.
>> 
>> https://github.com/eldraco19/evalute_improve_checkpatch_pl
>> 
>> Please let me know if there are any issues with this.
> 
> So far, so good.
> 
> Here are the questions we want to answer:
> 
> - So what are the 20 categories that occur most?
> 
> You are getting close to that answer, but you are not there yet.
> 
> Then look at the findings. For those 20 categories, are there specific
> findings that are multiple times false positives?
> 
> So, the script complains about something, but it does not get that the
> patch author wrote something completely unrelated to the error message.
> 
> Lukas

Sir,

I tried the given tasks and it can be found here,

https://github.com/eldraco19/evalute_improve_checkpatch_pl/blob/master/STATS.md

Please look into it.

Thanks,
Ayush
_______________________________________________
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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-08-21 16:18 ` Ayush
  2020-08-22  8:06   ` Lukas Bulwahn
  2020-08-24 10:06   ` Ayush
@ 2020-08-26 13:51   ` Ayush
  2 siblings, 0 replies; 16+ messages in thread
From: Ayush @ 2020-08-26 13:51 UTC (permalink / raw)
  To: Lukas Bulwahn; +Cc: linux-kernel-mentees

August 24, 2020 3:36 PM, "Ayush" <ayush@disroot.org> wrote:

> August 22, 2020 1:36 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
> 
>> On Fri, 21 Aug 2020, Ayush wrote:
>> 
>>> August 17, 2020 3:13 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
>>> 
>>> Dear Ayush,
>>> 
>>> The zeroth task is to learn suitable netiquette for the communication with
>>> the kernel community.
>>> 
>>> First, please do not top-post.
>>> 
>>> A: Because we read from top to bottom, left to right.
>>> Q: Why should I start my reply below the quoted text?
>>> 
>>> A: Because it messes up the order in which people normally read text.
>>> Q: Why is top-posting such a bad thing?
>>> 
>>> A: The lost context.
>>> Q: What makes top-posted replies harder to read than bottom-posted?
>>> 
>>> A: Yes.
>>> Q: Should I trim down the quoted part of an email to which I'm
>>> replying?
>>> 
>>> Second, please always CC: linux-kernel-mentees@lists.linuxfoundation.org.
>>> 
>>> Third, set up your email client according to the kernel community rules.
>>> 
>>> Then, the first task is to run checkpatch.pl on a few kernel patches and
>>> collect the results. When you have that, please share your script with
>>> me, e.g., in a github repository.
>>> 
>>> Hints to the first task:
>>> 
>>> Can you create a list of all non-merge commits that were added in the
>>> version v5.8 of the kernel, i.e., all non-merge commits that are in v5.8
>>> and not already in v5.7?
>>> 
>>> Can you share the script/command you executed and the resulting list on
>>> github?
>>> 
>>> Can you run your script on all commits of this list above and record
>>> all checkpatch.pl reports, and store them in your github repository?
>>> 
>>> Can you suggest ideas how to aggregate the findings and create a
>>> statistics? For example: Which type of error is reported most?
>>> Can you implement that idea?
>>> 
>>> I also suggest to have a look at
>>> the options ./scripts/checkpatch.pl --list-types and
>>> ./scripts/checkpatch.pl --show-types. The option --show-types changes
>>> the output of checkpatch.pl to list type identifiers, so it is easier
>>> to parse and aggregate the output.
>>> 
>>> Please also share the script you create for that purpose on your
>>> github repository.
>>> 
>>> The second task is to pick one warning that appears often and improve
>>> checkpatch.pl to handle that better and get it accepted by the kernel
>>> community.
>>> 
>>> Hints to the second task follow when the first task is solved.
>>> 
>>> If you fail on any of those tasks, you are out of the selection process.
>>> 
>>> Lukas
>>> 
>>> Sir,
>>> 
>>> I have attempted the task 1 and pushed the same to GitHub.
>>> 
>>> Please have a look and suggest improvements.
>>> 
>>> https://github.com/eldraco19/evalute_improve_checkpatch_pl
>>> 
>>> Please let me know if there are any issues with this.
>> 
>> So far, so good.
>> 
>> Here are the questions we want to answer:
>> 
>> - So what are the 20 categories that occur most?
>> 
>> You are getting close to that answer, but you are not there yet.
>> 
>> Then look at the findings. For those 20 categories, are there specific
>> findings that are multiple times false positives?
>> 
>> So, the script complains about something, but it does not get that the
>> patch author wrote something completely unrelated to the error message.
>> 
>> Lukas
> 
> Sir,
> 
> I tried the given tasks and it can be found here,
> 
> https://github.com/eldraco19/evalute_improve_checkpatch_pl/blob/master/STATS.md
> 
> Please look into it.
> 
> Thanks,
> Ayush
> _______________________________________________
> Linux-kernel-mentees mailing list
> Linux-kernel-mentees@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Hello Sir,

Please review my task 1 and please point any issues with my submission.

Thanks,
Ayush
_______________________________________________
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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-08-24 10:06   ` Ayush
@ 2020-08-27  5:28     ` Lukas Bulwahn
  2020-08-30 18:51     ` Ayush
  1 sibling, 0 replies; 16+ messages in thread
From: Lukas Bulwahn @ 2020-08-27  5:28 UTC (permalink / raw)
  To: Ayush; +Cc: linux-kernel-mentees



On Mon, 24 Aug 2020, Ayush wrote:

> August 22, 2020 1:36 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
> 
> > On Fri, 21 Aug 2020, Ayush wrote:
> > 
> >> Hints to the first task:
> >> 
> >> Can you create a list of all non-merge commits that were added in the
> >> version v5.8 of the kernel, i.e., all non-merge commits that are in v5.8
> >> and not already in v5.7?
> >> 
> >> Can you share the script/command you executed and the resulting list on
> >> github?
> >> 
> >> Can you run your script on all commits of this list above and record
> >> all checkpatch.pl reports, and store them in your github repository?
> >> 
> >> Can you suggest ideas how to aggregate the findings and create a
> >> statistics? For example: Which type of error is reported most?
> >> Can you implement that idea?
> >> 
> >> I also suggest to have a look at
> >> the options ./scripts/checkpatch.pl --list-types and
> >> ./scripts/checkpatch.pl --show-types. The option --show-types changes
> >> the output of checkpatch.pl to list type identifiers, so it is easier
> >> to parse and aggregate the output.
> >> 
> >> Please also share the script you create for that purpose on your
> >> github repository.
> >> 
> >> The second task is to pick one warning that appears often and improve
> >> checkpatch.pl to handle that better and get it accepted by the kernel
> >> community.
> >> 
> >> Hints to the second task follow when the first task is solved.
> >> 
> >> If you fail on any of those tasks, you are out of the selection process.
> >> 
> >> Lukas
> >> 
> >> Sir,
> >> 
> >> I have attempted the task 1 and pushed the same to GitHub.
> >> 
> >> Please have a look and suggest improvements.
> >> 
> >> https://github.com/eldraco19/evalute_improve_checkpatch_pl
> >> 
> >> Please let me know if there are any issues with this.
> > 
> > So far, so good.
> > 
> > Here are the questions we want to answer:
> > 
> > - So what are the 20 categories that occur most?
> > 
> > You are getting close to that answer, but you are not there yet.
> > 
> > Then look at the findings. For those 20 categories, are there specific
> > findings that are multiple times false positives?
> > 
> > So, the script complains about something, but it does not get that the
> > patch author wrote something completely unrelated to the error message.
> > 
> > Lukas
> 
> Sir,
> 
> I tried the given tasks and it can be found here,
> 
> https://github.com/eldraco19/evalute_improve_checkpatch_pl/blob/master/STATS.md
>

The solution is implemented a bit complicated, but well, at least, it 
works if I believe your report. (I only read the code, but did not run 
it.)

The goal now is to find a class of false positives and improve 
checkpatch.pl accordingly.

I suggest that you look at the specific DIFF_IN_COMMIT_MSG reported 
errors?

Provide a short assessment for each DIFF_IN_COMMIT_MSG error in the 
10 commits.

It should tell:
  - what lines in the commit message did checkpatch.pl complain about?
  - what is the pattern in the commit message?
  - does patch(1) really stumble over that pattern?
  - how would this pattern need to be provided to patch(1) so that it 
would stumble over it?
  - if no, why not?
  - can we change checkpatch.pl to not raise an error for such a 
situation? So, only raise an error when the pattern would really make 
patch stumble on it?

Depending on the evaluation, we might continue to improve checkpatch.pl 
for reporting this error type, or we decide to look at GIT_COMMIT_ID 
errors, where I can quickly spot some false positives.

Best regards,

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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-08-24 10:06   ` Ayush
  2020-08-27  5:28     ` Lukas Bulwahn
@ 2020-08-30 18:51     ` Ayush
  2020-08-31  5:14       ` Lukas Bulwahn
  2020-09-06  9:59       ` Ayush
  1 sibling, 2 replies; 16+ messages in thread
From: Ayush @ 2020-08-30 18:51 UTC (permalink / raw)
  To: Lukas Bulwahn; +Cc: linux-kernel-mentees

August 27, 2020 10:59 AM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:

> On Mon, 24 Aug 2020, Ayush wrote:
> 
>> August 22, 2020 1:36 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
>> 
>> On Fri, 21 Aug 2020, Ayush wrote:
>> 
>> Hints to the first task:
>> 
>> Can you create a list of all non-merge commits that were added in the
>> version v5.8 of the kernel, i.e., all non-merge commits that are in v5.8
>> and not already in v5.7?
>> 
>> Can you share the script/command you executed and the resulting list on
>> github?
>> 
>> Can you run your script on all commits of this list above and record
>> all checkpatch.pl reports, and store them in your github repository?
>> 
>> Can you suggest ideas how to aggregate the findings and create a
>> statistics? For example: Which type of error is reported most?
>> Can you implement that idea?
>> 
>> I also suggest to have a look at
>> the options ./scripts/checkpatch.pl --list-types and
>> ./scripts/checkpatch.pl --show-types. The option --show-types changes
>> the output of checkpatch.pl to list type identifiers, so it is easier
>> to parse and aggregate the output.
>> 
>> Please also share the script you create for that purpose on your
>> github repository.
>> 
>> The second task is to pick one warning that appears often and improve
>> checkpatch.pl to handle that better and get it accepted by the kernel
>> community.
>> 
>> Hints to the second task follow when the first task is solved.
>> 
>> If you fail on any of those tasks, you are out of the selection process.
>> 
>> Lukas
>> 
>> Sir,
>> 
>> I have attempted the task 1 and pushed the same to GitHub.
>> 
>> Please have a look and suggest improvements.
>> 
>> https://github.com/eldraco19/evalute_improve_checkpatch_pl
>> 
>> Please let me know if there are any issues with this.
>> 
>> So far, so good.
>> 
>> Here are the questions we want to answer:
>> 
>> - So what are the 20 categories that occur most?
>> 
>> You are getting close to that answer, but you are not there yet.
>> 
>> Then look at the findings. For those 20 categories, are there specific
>> findings that are multiple times false positives?
>> 
>> So, the script complains about something, but it does not get that the
>> patch author wrote something completely unrelated to the error message.
>> 
>> Lukas
>> 
>> Sir,
>> 
>> I tried the given tasks and it can be found here,
>> 
>> https://github.com/eldraco19/evalute_improve_checkpatch_pl/blob/master/STATS.md
> 
> The solution is implemented a bit complicated, but well, at least, it
> works if I believe your report. (I only read the code, but did not run
> it.)
> 
> The goal now is to find a class of false positives and improve
> checkpatch.pl accordingly.
> 
> I suggest that you look at the specific DIFF_IN_COMMIT_MSG reported
> errors?
> 
> Provide a short assessment for each DIFF_IN_COMMIT_MSG error in the
> 10 commits.
> 
> It should tell:
> - what lines in the commit message did checkpatch.pl complain about?
> - what is the pattern in the commit message?
> - does patch(1) really stumble over that pattern?
> - how would this pattern need to be provided to patch(1) so that it
> would stumble over it?
> - if no, why not?
> - can we change checkpatch.pl to not raise an error for such a
> situation? So, only raise an error when the pattern would really make
> patch stumble on it?
> 
> Depending on the evaluation, we might continue to improve checkpatch.pl
> for reporting this error type, or we decide to look at GIT_COMMIT_ID
> errors, where I can quickly spot some false positives.
> 
> Best regards,
> 
> Lukas

Sir,

I analysed the given error type and my analysis can be found here:

https://github.com/eldraco19/evalute_improve_checkpatch_pl/blob/master/DIFF_IN_COMMIT_MSG.md

Please review it.

Thanks.
Ayush
_______________________________________________
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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-08-30 18:51     ` Ayush
@ 2020-08-31  5:14       ` Lukas Bulwahn
  2020-09-06  9:59       ` Ayush
  1 sibling, 0 replies; 16+ messages in thread
From: Lukas Bulwahn @ 2020-08-31  5:14 UTC (permalink / raw)
  To: Ayush; +Cc: linux-kernel-mentees



On Sun, 30 Aug 2020, Ayush wrote:

> August 27, 2020 10:59 AM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
> 
> > On Mon, 24 Aug 2020, Ayush wrote:
> > 
> >> August 22, 2020 1:36 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
> >> 
> >> On Fri, 21 Aug 2020, Ayush wrote:
> >> 
> >> Hints to the first task:
> >> 
> >> Can you create a list of all non-merge commits that were added in the
> >> version v5.8 of the kernel, i.e., all non-merge commits that are in v5.8
> >> and not already in v5.7?
> >> 
> >> Can you share the script/command you executed and the resulting list on
> >> github?
> >> 
> >> Can you run your script on all commits of this list above and record
> >> all checkpatch.pl reports, and store them in your github repository?
> >> 
> >> Can you suggest ideas how to aggregate the findings and create a
> >> statistics? For example: Which type of error is reported most?
> >> Can you implement that idea?
> >> 
> >> I also suggest to have a look at
> >> the options ./scripts/checkpatch.pl --list-types and
> >> ./scripts/checkpatch.pl --show-types. The option --show-types changes
> >> the output of checkpatch.pl to list type identifiers, so it is easier
> >> to parse and aggregate the output.
> >> 
> >> Please also share the script you create for that purpose on your
> >> github repository.
> >> 
> >> The second task is to pick one warning that appears often and improve
> >> checkpatch.pl to handle that better and get it accepted by the kernel
> >> community.
> >> 
> >> Hints to the second task follow when the first task is solved.
> >> 
> >> If you fail on any of those tasks, you are out of the selection process.
> >> 
> >> Lukas
> >> 
> >> Sir,
> >> 
> >> I have attempted the task 1 and pushed the same to GitHub.
> >> 
> >> Please have a look and suggest improvements.
> >> 
> >> https://github.com/eldraco19/evalute_improve_checkpatch_pl
> >> 
> >> Please let me know if there are any issues with this.
> >> 
> >> So far, so good.
> >> 
> >> Here are the questions we want to answer:
> >> 
> >> - So what are the 20 categories that occur most?
> >> 
> >> You are getting close to that answer, but you are not there yet.
> >> 
> >> Then look at the findings. For those 20 categories, are there specific
> >> findings that are multiple times false positives?
> >> 
> >> So, the script complains about something, but it does not get that the
> >> patch author wrote something completely unrelated to the error message.
> >> 
> >> Lukas
> >> 
> >> Sir,
> >> 
> >> I tried the given tasks and it can be found here,
> >> 
> >> https://github.com/eldraco19/evalute_improve_checkpatch_pl/blob/master/STATS.md
> > 
> > The solution is implemented a bit complicated, but well, at least, it
> > works if I believe your report. (I only read the code, but did not run
> > it.)
> > 
> > The goal now is to find a class of false positives and improve
> > checkpatch.pl accordingly.
> > 
> > I suggest that you look at the specific DIFF_IN_COMMIT_MSG reported
> > errors?
> > 
> > Provide a short assessment for each DIFF_IN_COMMIT_MSG error in the
> > 10 commits.
> > 
> > It should tell:
> > - what lines in the commit message did checkpatch.pl complain about?
> > - what is the pattern in the commit message?
> > - does patch(1) really stumble over that pattern?
> > - how would this pattern need to be provided to patch(1) so that it
> > would stumble over it?
> > - if no, why not?
> > - can we change checkpatch.pl to not raise an error for such a
> > situation? So, only raise an error when the pattern would really make
> > patch stumble on it?
> > 
> > Depending on the evaluation, we might continue to improve checkpatch.pl
> > for reporting this error type, or we decide to look at GIT_COMMIT_ID
> > errors, where I can quickly spot some false positives.
> > 
> > Best regards,
> > 
> > Lukas
> 
> Sir,
> 
> I analysed the given error type and my analysis can be found here:
> 
> https://github.com/eldraco19/evalute_improve_checkpatch_pl/blob/master/DIFF_IN_COMMIT_MSG.md
>

Evaluation looks sound. Although, I cannot really see the analysis of all 
10 commits. You say the 10 commits fall into two classes, but how can 
anyone else judge this from your report?

I also do not fully understand your conclusion; to me, it seems to 
contradict itself. Fortunately, I think your analysis suggests that there 
is not a clear improvement to checkpatch.pl, as far as I see.

So, I do not think that this is a good starting point for a change of 
checkpatch.pl.

I suggest that you look at the error type GIT_COMMIT_ID. I have found some 
cases that seem to be suitable for improvement of the checkpatch.pl 
script.


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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-08-30 18:51     ` Ayush
  2020-08-31  5:14       ` Lukas Bulwahn
@ 2020-09-06  9:59       ` Ayush
  2020-09-07  7:38         ` Lukas Bulwahn
  2020-09-07 14:27         ` Ayush
  1 sibling, 2 replies; 16+ messages in thread
From: Ayush @ 2020-09-06  9:59 UTC (permalink / raw)
  To: Lukas Bulwahn; +Cc: linux-kernel-mentees

August 31, 2020 10:44 AM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:

> On Sun, 30 Aug 2020, Ayush wrote:
> 
>> August 27, 2020 10:59 AM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
>> 
>> On Mon, 24 Aug 2020, Ayush wrote:
>> 
>> August 22, 2020 1:36 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
>> 
>> On Fri, 21 Aug 2020, Ayush wrote:
>> 
>> Hints to the first task:
>> 
>> Can you create a list of all non-merge commits that were added in the
>> version v5.8 of the kernel, i.e., all non-merge commits that are in v5.8
>> and not already in v5.7?
>> 
>> Can you share the script/command you executed and the resulting list on
>> github?
>> 
>> Can you run your script on all commits of this list above and record
>> all checkpatch.pl reports, and store them in your github repository?
>> 
>> Can you suggest ideas how to aggregate the findings and create a
>> statistics? For example: Which type of error is reported most?
>> Can you implement that idea?
>> 
>> I also suggest to have a look at
>> the options ./scripts/checkpatch.pl --list-types and
>> ./scripts/checkpatch.pl --show-types. The option --show-types changes
>> the output of checkpatch.pl to list type identifiers, so it is easier
>> to parse and aggregate the output.
>> 
>> Please also share the script you create for that purpose on your
>> github repository.
>> 
>> The second task is to pick one warning that appears often and improve
>> checkpatch.pl to handle that better and get it accepted by the kernel
>> community.
>> 
>> Hints to the second task follow when the first task is solved.
>> 
>> If you fail on any of those tasks, you are out of the selection process.
>> 
>> Lukas
>> 
>> Sir,
>> 
>> I have attempted the task 1 and pushed the same to GitHub.
>> 
>> Please have a look and suggest improvements.
>> 
>> https://github.com/eldraco19/evalute_improve_checkpatch_pl
>> 
>> Please let me know if there are any issues with this.
>> 
>> So far, so good.
>> 
>> Here are the questions we want to answer:
>> 
>> - So what are the 20 categories that occur most?
>> 
>> You are getting close to that answer, but you are not there yet.
>> 
>> Then look at the findings. For those 20 categories, are there specific
>> findings that are multiple times false positives?
>> 
>> So, the script complains about something, but it does not get that the
>> patch author wrote something completely unrelated to the error message.
>> 
>> Lukas
>> 
>> Sir,
>> 
>> I tried the given tasks and it can be found here,
>> 
>> https://github.com/eldraco19/evalute_improve_checkpatch_pl/blob/master/STATS.md
>> 
>> The solution is implemented a bit complicated, but well, at least, it
>> works if I believe your report. (I only read the code, but did not run
>> it.)
>> 
>> The goal now is to find a class of false positives and improve
>> checkpatch.pl accordingly.
>> 
>> I suggest that you look at the specific DIFF_IN_COMMIT_MSG reported
>> errors?
>> 
>> Provide a short assessment for each DIFF_IN_COMMIT_MSG error in the
>> 10 commits.
>> 
>> It should tell:
>> - what lines in the commit message did checkpatch.pl complain about?
>> - what is the pattern in the commit message?
>> - does patch(1) really stumble over that pattern?
>> - how would this pattern need to be provided to patch(1) so that it
>> would stumble over it?
>> - if no, why not?
>> - can we change checkpatch.pl to not raise an error for such a
>> situation? So, only raise an error when the pattern would really make
>> patch stumble on it?
>> 
>> Depending on the evaluation, we might continue to improve checkpatch.pl
>> for reporting this error type, or we decide to look at GIT_COMMIT_ID
>> errors, where I can quickly spot some false positives.
>> 
>> Best regards,
>> 
>> Lukas
>> 
>> Sir,
>> 
>> I analysed the given error type and my analysis can be found here:
>> 
>> https://github.com/eldraco19/evalute_improve_checkpatch_pl/blob/master/DIFF_IN_COMMIT_MSG.md
> 
> Evaluation looks sound. Although, I cannot really see the analysis of all
> 10 commits. You say the 10 commits fall into two classes, but how can
> anyone else judge this from your report?
> 
> I also do not fully understand your conclusion; to me, it seems to
> contradict itself. Fortunately, I think your analysis suggests that there
> is not a clear improvement to checkpatch.pl, as far as I see.
> 
> So, I do not think that this is a good starting point for a change of
> checkpatch.pl.
> 
> I suggest that you look at the error type GIT_COMMIT_ID. I have found some
> cases that seem to be suitable for improvement of the checkpatch.pl
> script.
> 
> Lukas

Sir,

I have been looking for more improvements in checkpatch.pl, especially with GIT_COMMIT_ID.

I found that commits which mentioned "revert commits" in their description will get error
even if they follow the proper syntax.

for example: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20200903&id=e8a170ff9a3576730e43c0dbdd27b7cd3dc56848

In this example,
commit description has,

commit 193392ed9f69 ("Revert "drm/amd/display: add -msse2 to prevent Clang from emitting libcalls to undefined SW FP routines"")

which is correct as per the syntax, but checkpatch still gives an error.

So, I tried to fix this by:

---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 149518d2a6a7..01df2b9b2236 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2874,6 +2874,9 @@ sub process {
                                $rawlines[$linenr] =~ /^\s*([^"]+)"\)/;
                                $orig_desc .= " " . $1;
                                $hasparens = 1;
+                       } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(Revert "[^"]+[^"]")"\)/i) {
+                               $orig_desc = $1;
+                               $hasparens = 1;
                        }
 
                        ($id, $description) = git_commit_info($orig_commit,
---
(on linux next-20200903)

This patch fixes the issues with commits of the similar type given in the above example but some cases like

- commit 1234567890ab ("Revert
"foo bar"")

- commit 1234567890ab ("Revert "foo
bar"")

- commit 1234567890ab
("Revert "foo bar"")


basically the cases where next-line comes are not handled. But there can a lot of different patterns where next-line is coming, so do we add a separate if condition for all the patterns? or we just continue giving 
an error in case of next-line?

Please look into this.


Thanks,
Ayush
_______________________________________________
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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-09-06  9:59       ` Ayush
@ 2020-09-07  7:38         ` Lukas Bulwahn
  2020-09-07 14:27         ` Ayush
  1 sibling, 0 replies; 16+ messages in thread
From: Lukas Bulwahn @ 2020-09-07  7:38 UTC (permalink / raw)
  To: Ayush; +Cc: linux-kernel-mentees

On Sun, Sep 6, 2020 at 11:59 AM Ayush <ayush@disroot.org> wrote:
>
> August 31, 2020 10:44 AM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
>
> > On Sun, 30 Aug 2020, Ayush wrote:
> >
> >> August 27, 2020 10:59 AM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
> >>
> >> On Mon, 24 Aug 2020, Ayush wrote:
> >>
> >> August 22, 2020 1:36 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
> >>
> >> On Fri, 21 Aug 2020, Ayush wrote:
> >>
> >> Hints to the first task:
> >>
> >> Can you create a list of all non-merge commits that were added in the
> >> version v5.8 of the kernel, i.e., all non-merge commits that are in v5.8
> >> and not already in v5.7?
> >>
> >> Can you share the script/command you executed and the resulting list on
> >> github?
> >>
> >> Can you run your script on all commits of this list above and record
> >> all checkpatch.pl reports, and store them in your github repository?
> >>
> >> Can you suggest ideas how to aggregate the findings and create a
> >> statistics? For example: Which type of error is reported most?
> >> Can you implement that idea?
> >>
> >> I also suggest to have a look at
> >> the options ./scripts/checkpatch.pl --list-types and
> >> ./scripts/checkpatch.pl --show-types. The option --show-types changes
> >> the output of checkpatch.pl to list type identifiers, so it is easier
> >> to parse and aggregate the output.
> >>
> >> Please also share the script you create for that purpose on your
> >> github repository.
> >>
> >> The second task is to pick one warning that appears often and improve
> >> checkpatch.pl to handle that better and get it accepted by the kernel
> >> community.
> >>
> >> Hints to the second task follow when the first task is solved.
> >>
> >> If you fail on any of those tasks, you are out of the selection process.
> >>
> >> Lukas
> >>
> >> Sir,
> >>
> >> I have attempted the task 1 and pushed the same to GitHub.
> >>
> >> Please have a look and suggest improvements.
> >>
> >> https://github.com/eldraco19/evalute_improve_checkpatch_pl
> >>
> >> Please let me know if there are any issues with this.
> >>
> >> So far, so good.
> >>
> >> Here are the questions we want to answer:
> >>
> >> - So what are the 20 categories that occur most?
> >>
> >> You are getting close to that answer, but you are not there yet.
> >>
> >> Then look at the findings. For those 20 categories, are there specific
> >> findings that are multiple times false positives?
> >>
> >> So, the script complains about something, but it does not get that the
> >> patch author wrote something completely unrelated to the error message.
> >>
> >> Lukas
> >>
> >> Sir,
> >>
> >> I tried the given tasks and it can be found here,
> >>
> >> https://github.com/eldraco19/evalute_improve_checkpatch_pl/blob/master/STATS.md
> >>
> >> The solution is implemented a bit complicated, but well, at least, it
> >> works if I believe your report. (I only read the code, but did not run
> >> it.)
> >>
> >> The goal now is to find a class of false positives and improve
> >> checkpatch.pl accordingly.
> >>
> >> I suggest that you look at the specific DIFF_IN_COMMIT_MSG reported
> >> errors?
> >>
> >> Provide a short assessment for each DIFF_IN_COMMIT_MSG error in the
> >> 10 commits.
> >>
> >> It should tell:
> >> - what lines in the commit message did checkpatch.pl complain about?
> >> - what is the pattern in the commit message?
> >> - does patch(1) really stumble over that pattern?
> >> - how would this pattern need to be provided to patch(1) so that it
> >> would stumble over it?
> >> - if no, why not?
> >> - can we change checkpatch.pl to not raise an error for such a
> >> situation? So, only raise an error when the pattern would really make
> >> patch stumble on it?
> >>
> >> Depending on the evaluation, we might continue to improve checkpatch.pl
> >> for reporting this error type, or we decide to look at GIT_COMMIT_ID
> >> errors, where I can quickly spot some false positives.
> >>
> >> Best regards,
> >>
> >> Lukas
> >>
> >> Sir,
> >>
> >> I analysed the given error type and my analysis can be found here:
> >>
> >> https://github.com/eldraco19/evalute_improve_checkpatch_pl/blob/master/DIFF_IN_COMMIT_MSG.md
> >
> > Evaluation looks sound. Although, I cannot really see the analysis of all
> > 10 commits. You say the 10 commits fall into two classes, but how can
> > anyone else judge this from your report?
> >
> > I also do not fully understand your conclusion; to me, it seems to
> > contradict itself. Fortunately, I think your analysis suggests that there
> > is not a clear improvement to checkpatch.pl, as far as I see.
> >
> > So, I do not think that this is a good starting point for a change of
> > checkpatch.pl.
> >
> > I suggest that you look at the error type GIT_COMMIT_ID. I have found some
> > cases that seem to be suitable for improvement of the checkpatch.pl
> > script.
> >
> > Lukas
>
> Sir,
>
> I have been looking for more improvements in checkpatch.pl, especially with GIT_COMMIT_ID.
>
> I found that commits which mentioned "revert commits" in their description will get error
> even if they follow the proper syntax.
>
> for example: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20200903&id=e8a170ff9a3576730e43c0dbdd27b7cd3dc56848
>
> In this example,
> commit description has,
>
> commit 193392ed9f69 ("Revert "drm/amd/display: add -msse2 to prevent Clang from emitting libcalls to undefined SW FP routines"")
>
> which is correct as per the syntax, but checkpatch still gives an error.
>
> So, I tried to fix this by:
>
> ---
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 149518d2a6a7..01df2b9b2236 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2874,6 +2874,9 @@ sub process {
>                                 $rawlines[$linenr] =~ /^\s*([^"]+)"\)/;
>                                 $orig_desc .= " " . $1;
>                                 $hasparens = 1;
> +                       } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(Revert "[^"]+[^"]")"\)/i) {
> +                               $orig_desc = $1;
> +                               $hasparens = 1;
>                         }
>
>                         ($id, $description) = git_commit_info($orig_commit,
> ---
> (on linux next-20200903)
>

First, your email client is broken. Just to let you know. My client is
broken, too :) I did not bother now to pull out my working email
client...

Generally, you identified the right place to add this extended check
and the check makes sense.

> This patch fixes the issues with commits of the similar type given in the above example but some cases like
>
> - commit 1234567890ab ("Revert
> "foo bar"")
>
> - commit 1234567890ab ("Revert "foo
> bar"")
>
> - commit 1234567890ab
> ("Revert "foo bar"")
>
>
> basically the cases where next-line comes are not handled. But there can a lot of different patterns where next-line is coming, so do we add a separate if condition for all the patterns? or we just continue giving
> an error in case of next-line?
>

I think you should extend this whole check to work properly with line
breaks. You can see how this is implemented currently in checkpatch.pl
just a few lines above, right?

Can you provide a full list of checkpatch.pl findings in v5.4..v5.8
where all the different commit 1234567890ab ("Revert "..."") variants
in the commit message appear?

Then, we can use those commits directly as test cases for your extension?

Best regards,

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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-09-06  9:59       ` Ayush
  2020-09-07  7:38         ` Lukas Bulwahn
@ 2020-09-07 14:27         ` Ayush
  2020-09-07 16:43           ` Lukas Bulwahn
  2020-09-09  9:14           ` Ayush
  1 sibling, 2 replies; 16+ messages in thread
From: Ayush @ 2020-09-07 14:27 UTC (permalink / raw)
  To: Lukas Bulwahn; +Cc: linux-kernel-mentees

September 7, 2020 1:09 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
 
> I think you should extend this whole check to work properly with line
> breaks. You can see how this is implemented currently in checkpatch.pl
> just a few lines above, right?

I have written the conditions for handling line breaks.

I will be sending the patch to the mailing list after this mail.

> Can you provide a full list of checkpatch.pl findings in v5.4..v5.8
> where all the different commit 1234567890ab ("Revert "..."") variants
> in the commit message appear?
> 
> Then, we can use those commits directly as test cases for your extension?

Actually, I have made a list for some of the similar commits which are correct but
checkpatch.pl will give GIT_COMMIT_ID error on them. I have tested my patch with 30+ commits.

Here is the list of commits: 

https://gist.githubusercontent.com/eldraco19/f78e637ead0e47e1bb187f83a391a904/raw/1ee853ab1b7db86d663f6537b7c43e284e30dad2/commits.txt

Some doubts I still have:
- commit
1234567890ab ("Revert "foo bar"")

is not allowed as per the rules, right? 

- if the commit mentioned has double-quotes like:
commit 1234567890ab ("Revert "foo "lorem ipsum" bar"")

example: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.8.y&id=9d7c249a1ef9bf0d5696df14e6bc067004f16979

For now, checkpatch.pl is giving an error in such cases too. But there can be `n` cases like these, we 
can have so many quotes nested, so I think it's better to avoid this pattern, please give your opinions on this.

Thanks,
Ayush
_______________________________________________
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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-09-07 14:27         ` Ayush
@ 2020-09-07 16:43           ` Lukas Bulwahn
  2020-09-09  9:14           ` Ayush
  1 sibling, 0 replies; 16+ messages in thread
From: Lukas Bulwahn @ 2020-09-07 16:43 UTC (permalink / raw)
  To: Ayush; +Cc: linux-kernel-mentees

On Mon, Sep 7, 2020 at 4:28 PM Ayush <ayush@disroot.org> wrote:
>
> September 7, 2020 1:09 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
>
> > I think you should extend this whole check to work properly with line
> > breaks. You can see how this is implemented currently in checkpatch.pl
> > just a few lines above, right?
>
> I have written the conditions for handling line breaks.
>
> I will be sending the patch to the mailing list after this mail.
>
> > Can you provide a full list of checkpatch.pl findings in v5.4..v5.8
> > where all the different commit 1234567890ab ("Revert "..."") variants
> > in the commit message appear?
> >
> > Then, we can use those commits directly as test cases for your extension?
>
> Actually, I have made a list for some of the similar commits which are correct but
> checkpatch.pl will give GIT_COMMIT_ID error on them. I have tested my patch with 30+ commits.
>
> Here is the list of commits:
>
> https://gist.githubusercontent.com/eldraco19/f78e637ead0e47e1bb187f83a391a904/raw/1ee853ab1b7db86d663f6537b7c43e284e30dad2/commits.txt
>
> Some doubts I still have:
> - commit
> 1234567890ab ("Revert "foo bar"")
>
> is not allowed as per the rules, right?
>

No, that is allowed. There can be of course a line break after commit
and before the hash, it is just that the current checkpatch.pl script
does not handle this case yet.

That would need improvement.

> - if the commit mentioned has double-quotes like:
> commit 1234567890ab ("Revert "foo "lorem ipsum" bar"")
>
> example: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.8.y&id=9d7c249a1ef9bf0d5696df14e6bc067004f16979
>
> For now, checkpatch.pl is giving an error in such cases too. But there can be `n` cases like these, we
> can have so many quotes nested, so I think it's better to avoid this pattern, please give your opinions on this.
>

I do not think that it is disallowed, but maybe a new check in
checkpatch.pl is recommended to make aware that quotes in commit
message subject lines cannot be properly handled and are discouraged.

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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-09-07 14:27         ` Ayush
  2020-09-07 16:43           ` Lukas Bulwahn
@ 2020-09-09  9:14           ` Ayush
  2020-09-09  9:52             ` Lukas Bulwahn
  2020-09-09 16:01             ` Ayush
  1 sibling, 2 replies; 16+ messages in thread
From: Ayush @ 2020-09-09  9:14 UTC (permalink / raw)
  To: Lukas Bulwahn; +Cc: linux-kernel-mentees

> No, that is allowed. There can be of course a line break after commit
> and before the hash, it is just that the current checkpatch.pl script
> does not handle this case yet.
> 
> That would need improvement.

I have already sent a patch for handling quotes in commit messages.
(https://lists.linuxfoundation.org/pipermail/linux-kernel-mentees/2020-September/004026.html)

I have created another patch which fixes line breaks after commit and
before hash.

It can handles cases like:

- commit
1234567890ab ("foo bar")

- commit
1234567890ab ("Revert "foo bar"")

But, my question is, should I send this as a new patch or append it with
my last patch and make a patch v2?
My last patch is still under review.

> I do not think that it is disallowed, but maybe a new check in
> checkpatch.pl is recommended to make aware that quotes in commit
> message subject lines cannot be properly handled and are discouraged.

So, should we give a warning if (no. of quotes > 4)?


Regards,
Ayush
_______________________________________________
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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-09-09  9:14           ` Ayush
@ 2020-09-09  9:52             ` Lukas Bulwahn
  2020-09-09 16:01             ` Ayush
  1 sibling, 0 replies; 16+ messages in thread
From: Lukas Bulwahn @ 2020-09-09  9:52 UTC (permalink / raw)
  To: Ayush; +Cc: linux-kernel-mentees



On Wed, 9 Sep 2020, Ayush wrote:

> > No, that is allowed. There can be of course a line break after commit
> > and before the hash, it is just that the current checkpatch.pl script
> > does not handle this case yet.
> > 
> > That would need improvement.
> 
> I have already sent a patch for handling quotes in commit messages.
> (https://lists.linuxfoundation.org/pipermail/linux-kernel-mentees/2020-September/004026.html)
>

Yes, I have seen that. I do not think it will be accepted (it is just too 
unclear), but you will see.

> I have created another patch which fixes line breaks after commit and
> before hash.
> 
> It can handles cases like:
> 
> - commit
> 1234567890ab ("foo bar")
> 
> - commit
> 1234567890ab ("Revert "foo bar"")
> 
> But, my question is, should I send this as a new patch or append it with
> my last patch and make a patch v2?

Create a separate patch that can be applied independently. As a mentor, I 
suggest to first send the patch only to me and the linux-kernel-mentees 
list, so that you get some first feedback without bothering the 
maintainers and them losing patience to work with you.

I suggest to work on this patch here first and then revisit the other 
patch for revert commits you created.

> My last patch is still under review.
> 
> > I do not think that it is disallowed, but maybe a new check in
> > checkpatch.pl is recommended to make aware that quotes in commit
> > message subject lines cannot be properly handled and are discouraged.
> 
> So, should we give a warning if (no. of quotes > 4)?
> 

I suggest to implement a check if a commit message contains quotes. Then, 
we can check on previous commits how often that would trigger and see if 
we find legitimate cases where quotes really make sense; then we can 
improve checkpatch.pl to handle those well when refering to such commits.


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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-09-09  9:14           ` Ayush
  2020-09-09  9:52             ` Lukas Bulwahn
@ 2020-09-09 16:01             ` Ayush
  2020-09-10  6:04               ` Lukas Bulwahn
  1 sibling, 1 reply; 16+ messages in thread
From: Ayush @ 2020-09-09 16:01 UTC (permalink / raw)
  To: Lukas Bulwahn; +Cc: linux-kernel-mentees

September 9, 2020 3:22 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
 
> Create a separate patch that can be applied independently. As a mentor, I
> suggest to first send the patch only to me and the linux-kernel-mentees
> list, so that you get some first feedback without bothering the
> maintainers and them losing patience to work with you.


I created a patch which handles line break between commit and hash value,

It can be found here:

https://gist.github.com/eldraco19/1631a75f859a07541d3c7c73c4958ca1

Please review it and suggest any improvements.

Thanks,
Ayush
_______________________________________________
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] 16+ messages in thread

* Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl"
  2020-09-09 16:01             ` Ayush
@ 2020-09-10  6:04               ` Lukas Bulwahn
  0 siblings, 0 replies; 16+ messages in thread
From: Lukas Bulwahn @ 2020-09-10  6:04 UTC (permalink / raw)
  To: Ayush; +Cc: linux-kernel-mentees



On Wed, 9 Sep 2020, Ayush wrote:

> September 9, 2020 3:22 PM, "Lukas Bulwahn" <lukas.bulwahn@gmail.com> wrote:
>  
> > Create a separate patch that can be applied independently. As a mentor, I
> > suggest to first send the patch only to me and the linux-kernel-mentees
> > list, so that you get some first feedback without bothering the
> > maintainers and them losing patience to work with you.
> 
> 
> I created a patch which handles line break between commit and hash value,
> 
> It can be found here:
> 
> https://gist.github.com/eldraco19/1631a75f859a07541d3c7c73c4958ca1
> 
> Please review it and suggest any improvements.
>

Ayush, that is NOT how the review process in the kernel community works.

The process is You send a patch to a mailing list for review, reviewers 
review and send back emails with comments. Then, you rework and send out a 
new patch and it repeats. That is the process.

For mentees, we have the linux-kernel-mentees mailing list, so that you 
can send patches to reviewers that are willing to provide dedicated 
feedback to mentees.


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] 16+ messages in thread

end of thread, other threads:[~2020-09-10  6:04 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200810125354.xeijyh3v5upatrez@salamander>
2020-08-17  9:43 ` [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl" Lukas Bulwahn
2020-08-21 16:18 ` Ayush
2020-08-22  8:06   ` Lukas Bulwahn
2020-08-24 10:06   ` Ayush
2020-08-27  5:28     ` Lukas Bulwahn
2020-08-30 18:51     ` Ayush
2020-08-31  5:14       ` Lukas Bulwahn
2020-09-06  9:59       ` Ayush
2020-09-07  7:38         ` Lukas Bulwahn
2020-09-07 14:27         ` Ayush
2020-09-07 16:43           ` Lukas Bulwahn
2020-09-09  9:14           ` Ayush
2020-09-09  9:52             ` Lukas Bulwahn
2020-09-09 16:01             ` Ayush
2020-09-10  6:04               ` Lukas Bulwahn
2020-08-26 13:51   ` Ayush

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).