4 ms·

In OSS, after merging a PR, generally you can't get back at the author and make them do a follow-up change, because they are volunteers. A certain degree of gat

In OSS, after merging a PR, generally you can't get back at the author and make them do a follow-up change, because they are volunteers. A certain degree of gatekeeping is necessary. In a company however, you absolutely can just assign such a task to the author or someone else. That means it's possible to integrate changes into the main branch faster, which is very beneficial. I've seen teams that try to mimic OSS development and only merge absolutely perfect PRs, dragging out long-living branches and integration. At some point the perfectness can't keep up with the plan, huge PRs are suddenly reviewed hastily, and the end result is worse and slower. Bottomline: PRs can be used slightly differently in companies to improve speed and reduce overhead.

dherikb 4y ago
I have a similar opinion. Also if the team is mature enough, the PR can be reviewed after the merge, so the necessary changes can be made with the current code already integrated and already running in production. The downside for this is that the team needs to have some discipline to review the already merged PRs and the developers needs to be disciplined to organize your time to make the fixes after the PR merged.
schwartzworld 4y ago
Big PRs with lots of changes are a smell of their own. If you're seeing this cycle, then PRs are just too big and should be broken into smaller tasks. Of course you're going to miss stuff and have a long review cycle if you have 20+ files with meaningful changes in a PR.
- the_gipsy 4y ago
  I'm specifically talking about small (or reasonable) tasks that get blown up in the PR phase because of perfectionist reviewers. Requesting for example a big architectural change in a pull request is not a good idea IMO. Ideally it should have happened in the design phase, but overspecifying everything is not the solution. Now, in a company, a request for a bigger change can simply be mentioned in the PR, and the tasks can then be created and assigned as follow ups. This is usually not possible in OSS; the author of the PR might agree to the necessary changes but later not do them for whatever reason, and there might be no other volunteers to "clean up". Therefore it does makes sense to gatekeep and drag out some PRs in OSS.

dherikb 4y ago

I have a similar opinion. Also if the team is mature enough, the PR can be reviewed after the merge, so the necessary changes can be made with the current code already integrated and already running in production. The downside for this is that the team needs to have some discipline to review the already merged PRs and the developers needs to be disciplined to organize your time to make the fixes after the PR merged.

schwartzworld 4y ago

Big PRs with lots of changes are a smell of their own. If you're seeing this cycle, then PRs are just too big and should be broken into smaller tasks. Of course you're going to miss stuff and have a long review cycle if you have 20+ files with meaningful changes in a PR.

the_gipsy 4y ago
I'm specifically talking about small (or reasonable) tasks that get blown up in the PR phase because of perfectionist reviewers. Requesting for example a big architectural change in a pull request is not a good idea IMO. Ideally it should have happened in the design phase, but overspecifying everything is not the solution. Now, in a company, a request for a bigger change can simply be mentioned in the PR, and the tasks can then be created and assigned as follow ups. This is usually not possible in OSS; the author of the PR might agree to the necessary changes but later not do them for whatever reason, and there might be no other volunteers to "clean up". Therefore it does makes sense to gatekeep and drag out some PRs in OSS.