[nixgeek]

On the contrary, I think the feedback provided was excellent and far better than just saying “Doesn’t conform to our style guidelines, please try again”. Bravo Peter.

This may be someone’s first submission and they may consequently not be aware of style guides, tools which can help lint, etc?

[db48x]

Agreed. Also "missing {}" is a super important "style" comment.

[peterzz]

Because bugs due to missing {} happen, and are a royal pain, the rule is that any nontrivial statement; especially any multi-line; must have braces.

Also, given the amount of patches I have to read, uniform style really does matter. Unconventional style breaks the flow and detracts from the important bits.

Also, I'm not aware of a lint like tool that works on diffs. Many of the patches never get further than my MUA.

[db48x]

I think you took my comment as sarcasm, but I quite agree with you; being able to leave out the curly braces was a mistake in the language design. Not as big as some of the others, but costly enough over all these decades.

Personally I prefer to be less strict about style whenever possible, but then I prefer to work in safer languages, and I don't have the same firehose to deal with.

[eco]

The technical review was great. The tone of the review left a lot to be desired.

[ori_b]

The technical review wasn't technical. It was a human roleplaying as a code formatter. The technical content was entirely found in the comment about ABI compatiblity.

I didn't see any discussion about tradeoffs, alternative approaches, or a survey of what other systems do for this kind of functionality, or detailed benchmark results.

The tone was roughly what I'd want people to give me in a code review -- the only problem is that it was trivial, and could have been summarized as "Fix the style, check it with $tool"

[cesarb]

> The technical review wasn't technical. [...] The technical content was entirely found in the comment about ABI compatiblity.

That review also had a comment about an implicit limit on the number of objects, which is caused by a limit on the amount of physically contiguous memory the kernel memory allocator can obtain at once, and a comment that the code being reviewed would allow for a large increase of the reference count of a couple of important structures. Both appear to be very technical comments to me.

[Sahhaese]

This is my complaint about the styling issue, that the important technical notes get lost in the 'noise' of styling issues.

[CobrastanJorji]

From my understanding, this is a pretty solid summation of the entire history of the LKML.