On Mon, Feb 16, 2009 at 8:52 PM, Stefan Richter <stefanr@xxxxxxxxxxxxxxxxx> wrote: > Ingo Molnar wrote: >> We routinely mention Sparse, lockdep, Coverity, Coccinelle, kmemleak, >> ftrace, kmemcheck and other tools as well when it motives to fix a bug >> or uncleanliness. [...] It is absolutely fine to >> mention checkpatch when it catches uncleanliness in code that already >> got merged. I dont understand your point. > > I wrote "don't mention checkpatch" but I really meant "think about what > the effect of the patch is and describe this". > > It's not really a hard problem to mention checkpatch --- it is a problem > to make it the main point or, like in this case, the only point of the > changelog. (Furthermore, it is also a problem to do something routinely > *if* doing it does not make sense. There routinely appear coccinelle > metapatch sources in changelogs. That does not make sense at all, and > doing it routinely is not a justification in itself.) > > So, "don't mention checkpatch" is simply a rule of thumb; read it as "I > mentioned checkpatch in the changelog --- wait, I have possibly written > a changelog that is besides the point; I should think about it once > more". :-) > > Now, when this particular patch is updated to get a good changelog, then > the title could become e.g.: > kernel/kallsyms: change initcall level; adjust whitespace > and anything more than that is just fluff and wasted electrons. Actually > the changelog should rather contain a note on why device_initcall is > supposed to be the correct initcall level. > > Fixes due to reports from sparse, lockdep, coverity, coccinelle, etc. > are the in this respect the same as fixes due to reports from > checkpatch: Patch titles should for example be > - "fix potential deadlock..." > - "fix use-after free..." > - "use XYZ helper..." > - "adjust whitespace..." > and *not* something like "fix lockdep backtrace" or whatever. > > A difference would be a patch title like "add sparse annotations" > because this is indeed about what the patch does, not by which means it > was created. > > Why do I make a fuzz? Well, because many of our changelogs really suck > and we need to become better in general. Hi Stefan/Ingo/Sam and others, Thanks a lot for all your feedbacks. It's a new learning for me and I will ensure that I don't repeat the same mistakes next time. Will resend all the patches with a new subject and more sensible changelog. Thanks - Manish > -- > Stefan Richter > -=====-=-=== -=-= -==-= > http://arcgraph.de/sr/ > -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
