3 ms·
Sometimes it is only in the last 10-30% of the changes that any significant benefit is realized, yet the preceding work is a necessary foundation. That makes it
by xenadu02 5y ago
Sometimes it is only in the last 10-30% of the changes that any significant benefit is realized, yet the preceding work is a necessary foundation. That makes it a very difficult sell to the rest of the organization (whether OSS contributors or a corporation). Until the last bit it looks like you're making a neverending stream of pointless changes.
Large-scale changes also suffer from "too many cooks in the kitchen" and bikeshedding that can make getting the changes accepted a real slog. It is difficult to get people to buy into your vision purely with words, either because they don't see the value in it or they reflexively disagree that the benefits are achievable (sometimes motivated by their own burnout or cynical attitude).
The solution is to work on it in semi-secret until you can get things to a point where the value is demonstrated. That bypasses endless bikeshedding and pointless feedback rounds because everyone can see the value of the end state (if you've chosen something worthwhile to work on).
- ncann 5y agoIt's also a common pattern with code review: make a 100 lines change and you'll get at least 10 comments with endless bikeshedding, make a 100k lines change and people will say LGTM in no time.
- metabagel 5y agoLGTM = “Let’s Go To the Moon”?
- bee_rider 5y agoA 100k LGTM is the bad outcome for something important like the Linux Kernel, though, right?
- ncann 5y agoYes, but at the same time, I'm not sure how many people can thoroughly go through 2k commits and 100k diff for a single pull request. Most people, I assume, in that situation will just try to understand the main ideas behind the changes, scan through the areas that they're somewhat familiar with to see if there's any glaring issue, and trust that the highly-reputed maintainer who submitted the PR knew what he was doing.
- db48x 5y agoHow else would you divide the work up? We all need to go look at the commits on this branch that touch our favorite driver and make sure that they don’t break anything.
- plorkyeran 5y agoYeah, the review process for something like this is realistically that some maintainers look at the changes made to their specific subsystems, and if none of them spot problems you assume that the patchset as a whole is systemically correct. If any of them do spot problems that aren't entirely trivial then it gets more complicated.
- regularfry 5y agoA 100k LoC LGTM in any project whatever is not just a bad outcome, it's proof positive that whatever review process you've got is rubber-stamping and not worth the effort. The same applies at 100 LoC. And at 10.
- Arch-TK 5y agoIt depends on whether the change is split up into nice units or not. In this case, despite being a 250,000 line change, it's split up into 2300 patches. That's much easier to review and look at than the equivalent change in 5 patches. And I've seen the latter, it doesn't make me want to merge anything, it makes me want to kill someone.