Hacker News Reader

Comment by corndoge | Hacker News Reader

In a project like the kernel consistent style is important and the kernel has tools (Coccinelle spatches, checkpatch.pl) to help developers comply with it. There are standards that need to be followed and the bar is the same for everyone.

View on HN

But why can't the committer just reformat (and otherwise improve) the patch when accepting? Why make the extra round-trips to the submitter?

Because it takes a lot of time for maintainers to format every PR.

Why would someone accept a patch that did not follow publicly available guidelines?

Because it's not the committer's job to play housekeeper for the contributor. They have enough work to do already, and the contributor is the one who wants their code merged in the first place - they should put in the basic effort to clean their own code before submission, just like everybody else. And because that would introduce some grey areas into the Signed-off-by line[0]. Committers do not want to change submitted code. See the bit about process for maintainers modifying submitted patches and imagine if they had to do that all the time because people can't just follow the damn style guide.

[0] https://www.kernel.org/doc/html/v4.17/process/submitting-pat...

You haven't contribute to a large project or figured out its culture yet, have you? In most major free and open source projects, the default rule is: the entire burdens of correcting any issues, including code-formatting, is the sole responsibility of the contributor, some big projects on GitHub even integrated automatic checking, few people would bother to review a patch if it doesn't fit the coding standard or fails to compile). And no, it's not only used an excuse for disliking one's patches. Even long-term contributors often resubmit some patches to fix their coding styles. If you are a high-profile developer of a subsystem/project, perhaps sometimes you can get a generous help for a free typo/style-correction by the upper level committer (especially when the patch has already went through the review and started moving up, at this point, even the maintainers agree it's pointless to send the patch back), but in general, there's no such a thing.

Do you really suggest that ten reviewers (or however many they may be) do this work each and every one, because it is too much work for the sole submitter to do this once?

It is hard enough to get people to review code as it is. I think everyone would be better off with a little humility and be thankful that other people review their code, even in the cases where the review itself isn't very helpful.

After making those changes, they would need to then retest the code which they might not be able to do as thoroughly as the original author could

Reply on news.ycombinator.com