* Please do not generate patches purely based on checkpatch. @ 2015-07-25 19:50 Ahmed Soliman 2015-07-25 20:49 ` greg.freemyer at gmail.com 0 siblings, 1 reply; 7+ messages in thread From: Ahmed Soliman @ 2015-07-25 19:50 UTC (permalink / raw) To: kernelnewbies 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 ?? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Please do not generate patches purely based on checkpatch. 2015-07-25 19:50 Please do not generate patches purely based on checkpatch Ahmed Soliman @ 2015-07-25 20:49 ` greg.freemyer at gmail.com 2015-07-25 22:12 ` Greg KH 0 siblings, 1 reply; 7+ messages in thread From: greg.freemyer at gmail.com @ 2015-07-25 20:49 UTC (permalink / raw) To: kernelnewbies 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Please do not generate patches purely based on checkpatch. 2015-07-25 20:49 ` greg.freemyer at gmail.com @ 2015-07-25 22:12 ` Greg KH 2015-07-26 7:35 ` Yogesh Chaudhari 0 siblings, 1 reply; 7+ messages in thread From: Greg KH @ 2015-07-25 22:12 UTC (permalink / raw) To: kernelnewbies On Sat, Jul 25, 2015 at 04:49:32PM -0400, greg.freemyer at 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Please do not generate patches purely based on checkpatch. 2015-07-25 22:12 ` Greg KH @ 2015-07-26 7:35 ` Yogesh Chaudhari 2015-07-26 17:45 ` Greg KH 2015-07-26 19:32 ` Valdis.Kletnieks at vt.edu 0 siblings, 2 replies; 7+ messages in thread From: Yogesh Chaudhari @ 2015-07-26 7:35 UTC (permalink / raw) To: kernelnewbies 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Please do not generate patches purely based on checkpatch. 2015-07-26 7:35 ` Yogesh Chaudhari @ 2015-07-26 17:45 ` Greg KH 2015-07-26 19:32 ` Valdis.Kletnieks at vt.edu 1 sibling, 0 replies; 7+ messages in thread From: Greg KH @ 2015-07-26 17:45 UTC (permalink / raw) To: kernelnewbies 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Please do not generate patches purely based on checkpatch. 2015-07-26 7:35 ` Yogesh Chaudhari 2015-07-26 17:45 ` Greg KH @ 2015-07-26 19:32 ` Valdis.Kletnieks at vt.edu 2015-07-26 20:08 ` Yogesh Chaudhari 1 sibling, 1 reply; 7+ messages in thread From: Valdis.Kletnieks at vt.edu @ 2015-07-26 19:32 UTC (permalink / raw) To: kernelnewbies 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. -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 848 bytes Desc: not available Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150726/4bd9f07b/attachment.bin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Please do not generate patches purely based on checkpatch. 2015-07-26 19:32 ` Valdis.Kletnieks at vt.edu @ 2015-07-26 20:08 ` Yogesh Chaudhari 0 siblings, 0 replies; 7+ messages in thread From: Yogesh Chaudhari @ 2015-07-26 20:08 UTC (permalink / raw) To: kernelnewbies 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. > -------------- next part -------------- An HTML attachment was scrubbed... URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150726/6df2c72a/attachment.html ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-07-26 20:08 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-07-25 19:50 Please do not generate patches purely based on checkpatch Ahmed Soliman 2015-07-25 20:49 ` greg.freemyer at gmail.com 2015-07-25 22:12 ` Greg KH 2015-07-26 7:35 ` Yogesh Chaudhari 2015-07-26 17:45 ` Greg KH 2015-07-26 19:32 ` Valdis.Kletnieks at vt.edu 2015-07-26 20:08 ` Yogesh Chaudhari
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).