4 ms·
> But the adoption of Pull Requests as the primary way of contributing code has created problems. We’ve lost some of the “Ready to Ship” mentality we had when w
by chrsig 3y ago
> But the adoption of Pull Requests as the primary way of contributing code has created problems. We’ve lost some of the “Ready to Ship” mentality we had when we did Continuous Integration. Features-in-progress stay out of the way by delaying integration, and so we fall into the pitfalls of low-frequency integration that Continuous Integration sought to address.
Feature branches should be pulling/rebasing back on top of master, so the integrate smoothly into main.
> Sometimes Pull Requests sit around and get stale, or we’re not sure what to work on while we wait for review.
This is a sign to me that there's something going on along the lines of:
- the work isn't actually necessary / no one asked for it
- there are more pressing issues demanding others attention, and it's ok that it's sitting there for a while
- the team doesn't feel confident that their comments will be well received
- the team is dysfunctional and unable to communicate the need for a change to the PR
- the team doesn't understand review is one of their responsibilities
- the team is doing sprints, fills their sprint up with 40 hours/person of development time, then forgets to ponder where time for review will come from.
Fix the team communication problem, or anything else you do is also doomed, you just don't know how yet.
> Sometimes they become bloated as we think “well, I may as well do this while I’m here.”
So it hasn't gotten reviewed, may as well make it harder to review, that'll get it somewhere...
Once it reaches a certain size, you need to take a step back and consider how you can chunk it out to make it reviewable incrementally. If you can't do that, examine how it can be factored differently to allow incremental review.
> First – a trick to help you get the best of both worlds – merge your own pull request without waiting for feedback, then pay attention to the feedback when it comes.
Why are people in such a hurry to get it merged? If it's so trivial that it doesn't require a review, it's likely not so impactful that good engineering practice should be ignored.
Without review, I promise that the quality / prevalence of tests will decrease over time, and eventually you wont be able to trust that the one line change that you're making wont break something in production.
Patience and rigor help throughput. This article seems like it's advocating skipping around those because they require communication and get in the way of "getting things done" a. la. cowboy.
- srvaroa 3y agoThis. Most of the articles about working around code reviews give the same vibes as naive crypto enthusiasts towards financial regulation.