3 ms·

It’s a legitimate concern - I would not assume malice. How exactly would someone review a 25k loc .patch file?

by isatty 6y ago

mehrdadn 6y ago
I didn't mean to imply otherwise. Drama is often not malice and rather due to legitimate concerns on one or both sides.
michaelt 6y ago
It would be tough, no doubt. But it's not like splitting the feature into 100 patches of 250 lines each would make it any quicker to review. Or merging code that was known not to work, as it was only a fraction of what was needed for the functionality.
- wtallis 6y ago
  > But it's not like splitting the feature into 100 patches of 250 lines each would make it any quicker to review. Or merging code that was known not to work, as it was only a fraction of what was needed for the functionality. That would also be rejected, because the kernel maintainers aren't idiots and their standards aren't the stupid arbitrary rules you construe them to be. They generally want big changes to be broken up into logical, sensible chunks that each leave the tree in a usable state, so that git-bisect still works.
  - skissane 6y ago
    How do people merge big new filesystems in practice though? Especially one with years of pre-existing out-of-tree development? I guess one could start by merging a skeleton of the filesystem which supports mount/unmount but then returns an IO error on every operation? And then a patch to add directory traversal (you can view the files but not their contents), and then a patch to add file reading, and then a patch to add file writing, and then a patch to add mkdir/rmdir, and then a patch to add rename/delete of regular files. Breaking down an existing filesystem into a sequence of patches like that, no doubt it is doable, but it is going to be a lot of work.
    - wtallis 6y ago
      My guess is that given the history of this filesystem implementation, most of the review effort will be focused on the interface between this FS and the rest of the kernel. It's typical for all the changes touching communal files or introducing generic helper functions or data structures to be broken out into separate commits. If any of those helpers are a reinvention of stuff that's already in the kernel, there will need to be a justification for why NTFS needs its own special versions. It's not typical for a large patch series adding genuinely new stuff to be broken up into absurdly tiny commits. For the stuff that's truly internal to the filesystem implementation, it looks like one patch per file will be an acceptable granularity.
rat9988 6y ago
I assume malice because of the tone. The concern is legitimate, the tone is offputting.
- phs318u 6y ago
  Welcome to Linux kernel development. By lkml history, this tone is very mild. There are many examples of far worse commentary and personal attacks on devs. I’m not justifying this by the way. Linus can be a very smart jerk, and as a leader (THE leader) he sets the tone for what’s acceptable in the community. https://www.zdnet.com/article/linux-developer-who-took-on-linus-torvalds-over-abuse-quits-toxic-kernel-community/ https://www.zdnet.com/article/linux-developer-who-took-on-li...
deleted 6y ago
[deleted]

It’s a legitimate concern - I would not assume malice. How exactly would someone review a 25k loc .patch file?

by isatty 6y ago

It’s a legitimate concern - I would not assume malice. How exactly would someone review a 25k loc .patch file?

mehrdadn 6y ago

I didn't mean to imply otherwise. Drama is often not malice and rather due to legitimate concerns on one or both sides.

michaelt 6y ago

It would be tough, no doubt. But it's not like splitting the feature into 100 patches of 250 lines each would make it any quicker to review. Or merging code that was known not to work, as it was only a fraction of what was needed for the functionality.

wtallis 6y ago
> But it's not like splitting the feature into 100 patches of 250 lines each would make it any quicker to review. Or merging code that was known not to work, as it was only a fraction of what was needed for the functionality. That would also be rejected, because the kernel maintainers aren't idiots and their standards aren't the stupid arbitrary rules you construe them to be. They generally want big changes to be broken up into logical, sensible chunks that each leave the tree in a usable state, so that git-bisect still works.
- skissane 6y ago
  How do people merge big new filesystems in practice though? Especially one with years of pre-existing out-of-tree development? I guess one could start by merging a skeleton of the filesystem which supports mount/unmount but then returns an IO error on every operation? And then a patch to add directory traversal (you can view the files but not their contents), and then a patch to add file reading, and then a patch to add file writing, and then a patch to add mkdir/rmdir, and then a patch to add rename/delete of regular files. Breaking down an existing filesystem into a sequence of patches like that, no doubt it is doable, but it is going to be a lot of work.
  - wtallis 6y ago
    My guess is that given the history of this filesystem implementation, most of the review effort will be focused on the interface between this FS and the rest of the kernel. It's typical for all the changes touching communal files or introducing generic helper functions or data structures to be broken out into separate commits. If any of those helpers are a reinvention of stuff that's already in the kernel, there will need to be a justification for why NTFS needs its own special versions. It's not typical for a large patch series adding genuinely new stuff to be broken up into absurdly tiny commits. For the stuff that's truly internal to the filesystem implementation, it looks like one patch per file will be an acceptable granularity.

rat9988 6y ago

I assume malice because of the tone. The concern is legitimate, the tone is offputting.

phs318u 6y ago
Welcome to Linux kernel development. By lkml history, this tone is very mild. There are many examples of far worse commentary and personal attacks on devs. I’m not justifying this by the way. Linus can be a very smart jerk, and as a leader (THE leader) he sets the tone for what’s acceptable in the community. https://www.zdnet.com/article/linux-developer-who-took-on-linus-torvalds-over-abuse-quits-toxic-kernel-community/ https://www.zdnet.com/article/linux-developer-who-took-on-li...

deleted 6y ago

[deleted]