Etiquette of submitting patches for fixing coding style.
I watched Kroah-Hartman's video[1] on submitting patches where he walks through the process of fixing coding style. I feel like this would be a good way for me to jump into the process but I need more information. At what point do code style patches stop being more trouble then they're worth to the maintainers? For example, running checkpatch.pl against all files is showing around 3 non-trivial style violations per file. Would a patch fixing 12 violations across 4 files be worth submitting? 1. http://www.youtube.com/watch?v=LLBrBBImJt4
On Fri, Oct 26, 2012 at 1:57 AM, Eugene Voronkov <eugene.voronkov@gmail.com>wrote:
I watched Kroah-Hartman's video[1] on submitting patches where he walks through the process of fixing coding style. I feel like this would be a good way for me to jump into the process but I need more information. At what point do code style patches stop being more trouble then they're worth to the maintainers? For example, running checkpatch.pl against all files is showing around 3 non-trivial style violations per file. Would a patch fixing 12 violations across 4 files be worth submitting?
Suggestion would be, divide your single patch in separate individual patches with respect to functionality/violations fix.
1. http://www.youtube.com/watch?v=LLBrBBImJt4
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
On Fri, Oct 26, 2012 at 5:38 AM, Sumeet pawnikar <sumeet4linux@gmail.com>wrote:
On Fri, Oct 26, 2012 at 1:57 AM, Eugene Voronkov < eugene.voronkov@gmail.com> wrote:
I watched Kroah-Hartman's video[1] on submitting patches where he walks through the process of fixing coding style. I feel like this would be a good way for me to jump into the process but I need more information. At what point do code style patches stop being more trouble then they're worth to the maintainers? For example, running checkpatch.pl against all files is showing around 3 non-trivial style violations per file. Would a patch fixing 12 violations across 4 files be worth submitting?
1. http://www.youtube.com/watch?v=LLBrBBImJt4
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Suggestion would be, divide your single patch in separate individual patches with respect to functionality/violations fix.
So for example, one patch removes braces from if/else conditionals with single statement. Another patch fixes incorrect spacing. Correct?
Hi Eugene, On Oct 27, 2012 7:18 AM, "Eugene Voronkov" <eugene.voronkov@gmail.com> wrote:
On Fri, Oct 26, 2012 at 5:38 AM, Sumeet pawnikar <sumeet4linux@gmail.com>
wrote:
On Fri, Oct 26, 2012 at 1:57 AM, Eugene Voronkov <
eugene.voronkov@gmail.com> wrote:
I watched Kroah-Hartman's video[1] on submitting patches where he walks
through the process of fixing coding style. I feel like this would be a good way for me to jump into the process but I need more information. At what point do code style patches stop being more trouble then they're worth to the maintainers? For example, running checkpatch.pl against all files is showing around 3 non-trivial style violations per file. Would a patch fixing 12 violations across 4 files be worth submitting?
1. http://www.youtube.com/watch?v=LLBrBBImJt4
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Suggestion would be, divide your single patch in separate individual patches with respect to functionality/violations fix.
So for example, one patch removes braces from if/else conditionals with single statement. Another patch fixes incorrect spacing. Correct?
Best to fix all checkpatch errors and warnings in the same file in the same patch. If you ate fixing two related files (two files in the same driver for example) then I would put them both in the same patch as well. If the two files are unrelated, keep the patches separate. Don't mix functional changes with cosmetic (checkpatch) fixups. Regards, Graeme
On Thu, Oct 25, 2012 at 4:27 PM, Eugene Voronkov <eugene.voronkov@gmail.com> wrote:
I watched Kroah-Hartman's video[1] on submitting patches where he walks through the process of fixing coding style. I feel like this would be a good way for me to jump into the process but I need more information. At what point do code style patches stop being more trouble then they're worth to the maintainers? For example, running checkpatch.pl against all files is showing around 3 non-trivial style violations per file. Would a patch fixing 12 violations across 4 files be worth submitting?
Before spending time on this, read the email on the ext4 list from a couple weeks ago. http://marc.info/?l=linux-ext4&m=135048406513682&w=2 Basically pure checkpatch.pl generated patches are discouraged by a lot of maintainers. They break existing out of tree patches that people may be working on. The solution is to use checkpatch.pl when you are already working in a relevant code area. Then it becomes: 0/2 This is a patch series to fix such and such bug 1/2 checkpatch.pl patch to clean up the formatting of the files I'm working on. 2/2 patch to fix the bug I see that sequence all the time and the checkpatch cleanup is always taken. But a sequence of purely checkpatch cleanups will likely be rejected. fyi: Robert Day just asked for help cleaning up the Doc Book stuff. Doc Book pull comments out of the .c files and creates documentation. To clean it up, patches to the source files will be required. These are more likely to be accepted. So you could to a doc book series like: 0/2 a patch series to correct the documentation for xyz subsystem 1/2 checkpatch cleanup of the 2 files with doc updates 2/2 doc updates That pairing may actually get accepted. (I can't say I remember it being tried.) Greg
On Fri, Oct 26, 2012 at 5:45 PM, Greg Freemyer <greg.freemyer@gmail.com>wrote:
On Thu, Oct 25, 2012 at 4:27 PM, Eugene Voronkov <eugene.voronkov@gmail.com> wrote:
I watched Kroah-Hartman's video[1] on submitting patches where he walks through the process of fixing coding style. I feel like this would be a good way for me to jump into the process but I need more information. At what point do code style patches stop being more trouble then they're worth to the maintainers? For example, running checkpatch.pl against all files is showing around 3 non-trivial style violations per file. Would a patch fixing 12 violations across 4 files be worth submitting?
Before spending time on this, read the email on the ext4 list from a couple weeks ago.
http://marc.info/?l=linux-ext4&m=135048406513682&w=2
Basically pure checkpatch.pl generated patches are discouraged by a lot of maintainers.
They break existing out of tree patches that people may be working on.
The solution is to use checkpatch.pl when you are already working in a relevant code area.
Then it becomes:
0/2 This is a patch series to fix such and such bug 1/2 checkpatch.pl patch to clean up the formatting of the files I'm working on. 2/2 patch to fix the bug
I see that sequence all the time and the checkpatch cleanup is always taken.
But a sequence of purely checkpatch cleanups will likely be rejected.
fyi: Robert Day just asked for help cleaning up the Doc Book stuff.
Doc Book pull comments out of the .c files and creates documentation. To clean it up, patches to the source files will be required. These are more likely to be accepted.
So you could to a doc book series like:
0/2 a patch series to correct the documentation for xyz subsystem 1/2 checkpatch cleanup of the 2 files with doc updates 2/2 doc updates
That pairing may actually get accepted. (I can't say I remember it being tried.)
Greg
Yea, seems like they would cause more problems than they solve. Thanks for the heads up. I've been looking at those.
On Fri, 26 Oct 2012, Greg Freemyer wrote:
fyi: Robert Day just asked for help cleaning up the Doc Book stuff.
Doc Book pull comments out of the .c files and creates documentation. To clean it up, patches to the source files will be required. These are more likely to be accepted.
So you could to a doc book series like:
0/2 a patch series to correct the documentation for xyz subsystem 1/2 checkpatch cleanup of the 2 files with doc updates 2/2 doc updates
if you want to get started making fixes and submitting patches, i suggest doc fixes as an easy way to jump in for the simple reason that just making changes to documentation shouldn't break anything. :-) there is a separate list just for kernel documentation: http://vger.kernel.org/vger-lists.html#linux-doc so you know you'll be on-topic there chatting about documentation. from the perspective of someone who has no *official* connection with the kernel but who's submitted lots of pedantic patches over the years, let me make a couple suggestions. first, a *lot* of what's under the Documentation/ directory is absurdly old and worthless, and should probably be just deleted. ignore that stuff. other stuff under there is perfectly up-to-date, and should be left alone. the middle ground is the stuff that can be updated to reflect the current kernel, so pick a single file under there that reflects something you're interested in, start going through it, make corrections, and submit the final patch to the linux-doc list. try to work with a single file or topic at a time, it's easier to get your patches accepted. post on the kernel-doc stuff coming shortly, for the interested. rday -- ======================================================================== Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ========================================================================
On Sat, Oct 27, 2012 at 10:09 AM, Robert P. J. Day <rpjday@crashcourse.ca>wrote:
On Fri, 26 Oct 2012, Greg Freemyer wrote:
fyi: Robert Day just asked for help cleaning up the Doc Book stuff.
Doc Book pull comments out of the .c files and creates documentation. To clean it up, patches to the source files will be required. These are more likely to be accepted.
So you could to a doc book series like:
0/2 a patch series to correct the documentation for xyz subsystem 1/2 checkpatch cleanup of the 2 files with doc updates 2/2 doc updates
if you want to get started making fixes and submitting patches, i suggest doc fixes as an easy way to jump in for the simple reason that just making changes to documentation shouldn't break anything. :-) there is a separate list just for kernel documentation:
http://vger.kernel.org/vger-lists.html#linux-doc
so you know you'll be on-topic there chatting about documentation. from the perspective of someone who has no *official* connection with the kernel but who's submitted lots of pedantic patches over the years, let me make a couple suggestions.
first, a *lot* of what's under the Documentation/ directory is absurdly old and worthless, and should probably be just deleted. ignore that stuff. other stuff under there is perfectly up-to-date, and should be left alone.
the middle ground is the stuff that can be updated to reflect the current kernel, so pick a single file under there that reflects something you're interested in, start going through it, make corrections, and submit the final patch to the linux-doc list. try to work with a single file or topic at a time, it's easier to get your patches accepted.
post on the kernel-doc stuff coming shortly, for the interested.
rday
--
======================================================================== Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca
Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ========================================================================
Thanks I'll be following the documentation mailing list.
participants (5)
-
Eugene Voronkov -
Graeme Russ -
Greg Freemyer -
Robert P. J. Day -
Sumeet pawnikar