Hacker News Reader

Comment by SloopJon | Hacker News Reader

It's an interesting question as to which is more readable:

    -module_param(max_sets, int, 0600);
    +module_param(max_sets, int, S_IRUSR | S_IWUSR);

However, I'm missing context. Did someone at Intel really spam 1,285 patches without any prior discussion?

View on HN

There are some mixed opinions in the maintainers' replies. Steve Rostadt is NACKing all of his patches (amusingly also spamming everyone in the process) since he likes the octal and others are making the same argument. Other again are suggesting more use of S_IRUGO since the author seems to have broken up a large number of octal macros into naive combinations of the S_* macros without much thought for semantic intention. A hardware maintainer encouraged the use of some ATTR_RW macros which indicates that the author didn't necessarily recognise the preferred tropes for various subsystems.

The backdoor argument (mentioned in an earlier email in the OP thread) is pretty reasonable to raise. The maintainers seems to be fairly on top of things though, acking and nacking as patches come in but this will take a while to process. I suspect very much that these commits will have to be rewritten to include subsystem labels and more accurate summaries by the maintainers if they want to include them. The patch set as an aggregate whole is garbage.

Seems like it. I would hope Intel wouldn't hire someone that would think this is OK ... OTOH see Uber's writeup about how developers can't be expected to think about transactions.

Maybe there's some perverse incentive at Intel? Like performance review that considers how many patches you've sent? Or maybe its a broken tool to help submit patches?

I'd be inclined to think it was a commit per smaller change set and each commit got turned into a separate patch.

It looks like it got turned into one commit per file. Which is pretty atrocious.

Isn't "one commit per independent change" common advice in git tutorials? Each file can be changed independently of any other, so taken literally, exactly how you're supposed to do it.

That's wildly incorrect. "One commit per logical change" would mean that every single file modified in this way should have been in a single commit. By your logic, you could never have two files changes in the same commit.

> you could never have two files changes in the same commit.

We're going OT here, but i don't think that applies to C, where a "logical change" to a function definition may be split over two files (.c/.h).

In the kernel, the proper practice for sweeping changes like this is one commit per subsystem.

Reply on news.ycombinator.com