Hacker News new | ask | show | jobs
by ZephyrBlu 1116 days ago
The OP is talking about feature work, as am I.

Obviously if you make a change to something like your type system it's going to generate a very large diff, but you also aren't going to review the full diff.

You're just going to make the change with find+replace or some other automation then write in the PR description "I made this change to the type system". No one is actually reviewing 17k LOC.

1 comments

Actually the example is pretty good for the kind of problem I'm talking about.

Let's imagine that instead of optimizing the pointers to in-place structs, we were taking the optimized program and adding support for dynamically allocated atoms because of some new feature for dynamically adding/removing atoms.

We could of course split the value->pointer 17k line change into a single PR. But, that PR is only doing a pessimization of the code. On its own, it makes no sense and should be rejected. It only makes sense if I know it will be followed by the other feature, and even then, I would have to see the specific changes being made to know if this pessimization is worth it.

And if it got committed to the main branch, in preparation for other PRs that depend on it, the main branch is no longer in a releasable state, since it would be crazy to release with a performance penalty and no feature gain.

So, the right way to push this is as a single PR with a 17k-changed-LoC commit + the commits that actually use it. Of course, people would only manually review the other changes, but that's easy to do even if it's all in a single PR. And anyone looking back at history would clearly see that the pessimization was only a part of the dynamically-allocated atom feature, not a crazy change that someone did.

You specifically mention "big feature" in your original comment so it's confusing that this is a good example of the kind of problem you're talking about.

This is a very different situation than large PRs containing feature code. I think most people would agree that one large PR is the correct approach for this kind of situation.

Usually new features require modifications of old code, at least in my experience. If I came across as claiming it's likely for a feature to require 5k new lines of code, then I clearly communicated badly. But a feature coming with 5k lines of modified code, while rare, still happens several times a year in a large project in my experience.