Messages in this thread | | | From | Ani Sinha <> | Date | Mon, 19 Jul 2021 13:38:26 +0530 (IST) | Subject | Re: [PATCH v3] checkpatch: add a rule to check general block comment style |
| |
On Mon, 19 Jul 2021, Joe Perches wrote:
> On Mon, 2021-07-19 at 12:25 +0530, Ani Sinha wrote: > > > I do not see why we cannot add this rule to checkpatch. If the > > reviewer likes the other style of commenting they can always ask for a > > correction. Having checkpatch agree with Linus' preferred style of > > commenting and the preferred documeted style of commenting (which seems to > > be the same) does make everything uniform and agreeable. > > Too many novice developers take checkpatch output as dicta. > > It's not. >
Well those "novice" developers have perhaps worked in companies where tooling like pre-commit sanity hooks have provided immense value in ensuring certain basic rules and code quality is maintained across the board, particulay when the company scales. Existing violations did not deter them from adding stricter rules to make sure all future commits follow certain patterns. Ofcourse at the end of the day, common sense trumps any tooling, goes without saying.
> It's just produces suggestions that should _always_ be taken > not very seriously. Those suggestions should perhaps be > considered, but good taste should always override a brainless > script.
At the very very least, checkpatch should lay this out in clear terms every time this is run. Different communities have different rules and for me, I always run all my patches through checkpatch to make sure that the patch I sent out of review at least is checkpatch clean. This makes sure that I am not violating any obvious code submission rules laid out by that community. This is particularly true for kernel community where flaming people for even small issues seems to be the culture!
> > _Very_ few senior developers really care that much about any > particular comment style.
I disagree on this.
> > These are the same senior developers that would be burdened > with unnecessary patches to review from those novice developers > that believe checkpatch should always be followed. >
Well for those "novice" developers, the doc tells us to run checkpatch and address the complaints :
Are you sure your patch is free of silly mistakes? You should always run patches through scripts/checkpatch.pl and address the complaints it comes up with.
Anyways it seems this conversation is self serving for the kernel's sr developers so that they can take any stance convenient to them. There is no value.
| |