Please do not generate patches purely based on checkpatch.
I have sent a patch for cleaning about 40 error and 50 warning generated checkpatch to the maintainer and all what I got in responce is "Nack. Please do not generate patches purely based on checkpatch." so what did I do wrong ?! should I follow checkpatch or not ??
On July 25, 2015 3:50:30 PM EDT, Ahmed Soliman <ahmedsoliman0x666@gmail.com> wrote:
I have sent a patch for cleaning about 40 error and 50 warning generated checkpatch to the maintainer and all what I got in responce is "Nack. Please do not generate patches purely based on checkpatch." so what did I do wrong ?! should I follow checkpatch or not ??
If you want to practice the submission process the staging tree maintainer accepts checkpatch only fixes as patches. Most maintainers feel the cost of accepting the and breaking other people's pending patches is too expensive. Alternatively, if there are no pending patches then they will likely feel the code is stable so why risk a change for no real reason. On the other hand if you submit a patch that addresses a real bug, then simultaneously doing a checkpatch related patch to the same area is a very good idea. Greg -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
On Sat, Jul 25, 2015 at 04:49:32PM -0400, greg.freemyer@gmail.com wrote:
On the other hand if you submit a patch that addresses a real bug, then simultaneously doing a checkpatch related patch to the same area is a very good idea.
No, that would be two different things. Do the bug fix first, and then the cleanup on a different patch. And even then, most maintainers will not take a cleanup patch. Stick with subsystems that do take these types of fixes if you want/like to do them (i.e. drivers/staging/*) thanks, greg k-h
On 26 July 2015 at 03:42, Greg KH <greg@kroah.com> wrote:
No, that would be two different things. Do the bug fix first, and then the cleanup on a different patch. And even then, most maintainers will not take a cleanup patch. Stick with subsystems that do take these types of fixes if you want/like to do them (i.e. drivers/staging/*)
I have come to know that Greg is one of the most liberal maintainers in this regard and accepts checkpatch related patches, but other than that, it seems to depend on maintainer's choice (which is fine IMHO). However, is there a place which documents which maintainers(and/or sub-systems) accept checkpatch(or other cleanup related) patches and who will reject them outright? Wouldn't it be good to have this documented, especially given that using the checkpatch is advised in Documentation/SubmitChecklist? Thanks Yogesh
On Sun, Jul 26, 2015 at 01:05:37PM +0530, Yogesh Chaudhari wrote:
On 26 July 2015 at 03:42, Greg KH <greg@kroah.com> wrote:
No, that would be two different things. Do the bug fix first, and then the cleanup on a different patch. And even then, most maintainers will not take a cleanup patch. Stick with subsystems that do take these types of fixes if you want/like to do them (i.e. drivers/staging/*)
I have come to know that Greg is one of the most liberal maintainers in this regard and accepts checkpatch related patches, but other than that, it seems to depend on maintainer's choice (which is fine IMHO). However, is there a place which documents which maintainers(and/or sub-systems) accept checkpatch(or other cleanup related) patches and who will reject them outright? Wouldn't it be good to have this documented, especially given that using the checkpatch is advised in Documentation/SubmitChecklist?
checkpatch is required for when you submit new patches, cleaning up existing code using checkpatch is not a good idea unless you are sending patches in for drivers/staging/* So never use the --file option, unless you know for sure that the maintainer accepts such patches. And if you don't know the answer to that, assume that they do not :) thanks, greg k-h
On Sun, 26 Jul 2015 13:05:37 +0530, Yogesh Chaudhari said:
However, is there a place which documents which maintainers(and/or sub-systems) accept checkpatch(or other cleanup related) patches and who will reject them outright? Wouldn't it be good to have this documented, especially given that using the checkpatch is advised in Documentation/SubmitChecklist?
Try something like 'git log drivers/net/wireless/whateverdev' (or whatever part of the tree you're looking at, and see if there have been previous checkpatch fixes in that part of the tree.
Thanks Valdis. Seems like a d'oh moment now. Never thought of scanning through the git history for checkpatch . Good tip. Thanks and regards Yogesh On Mon, Jul 27, 2015 at 1:02 AM <Valdis.Kletnieks@vt.edu> wrote:
On Sun, 26 Jul 2015 13:05:37 +0530, Yogesh Chaudhari said:
However, is there a place which documents which maintainers(and/or sub-systems) accept checkpatch(or other cleanup related) patches and who will reject them outright? Wouldn't it be good to have this documented, especially given that using the checkpatch is advised in Documentation/SubmitChecklist?
Try something like 'git log drivers/net/wireless/whateverdev' (or whatever part of the tree you're looking at, and see if there have been previous checkpatch fixes in that part of the tree.
participants (5)
-
Ahmed Soliman -
Greg KH -
greg.freemyer＠gmail.com -
Valdis.Kletnieks＠vt.edu -
Yogesh Chaudhari