Claim: the ideal PR is 50 lines long
graphite.dev
Claim: the ideal PR is 50 lines long
1–10 of 127 posts
Re: Claim: the ideal PR is 50 lines long
#2Forget about refactoring then
Re: Claim: the ideal PR is 50 lines long
#3... or is constrained to doing a single logical thing (perhaps many times).
Re: Claim: the ideal PR is 50 lines long
#4Forget about refactoring then
FTA:
Once PRs start to exceed 10k lines of code, they seem to become slightly “safer.”
I suspect this is because the extreme end of PR sizes includes refactors, which
maybe start including less functionality change and therefore have a slightly
lower chance of breaking. Alternatively, engineers may become progressively more
reluctant to revert PRs after 10k lines because of emotional anchoring and merge
conflicts.Re: Claim: the ideal PR is 50 lines long
#5I wish. My small PRs die by a thousand bikesheds. My large refactors are followed by LGTMYOLO
Re: Claim: the ideal PR is 50 lines long
#6> 50-line code changes are reviewed and merged ~40% faster than 250-line changes.
Reviewing a 50-line code change in 60% of the time it takes to review a 250-line code change means the shorter code review takes _four times_ as long to review per line.
Re: Claim: the ideal PR is 50 lines long
#7I’m reading this article on a break from reviewing a PR with more than 300 lines changed over 20+ files and I wonder if some people simply live in some sort of alternative wonderland universe.
The “ideals” are almost never a thing I see in my daily job, and I’ve worked for countless companies as a contractor.
Re: Claim: the ideal PR is 50 lines long
#8Forget about refactoring then
FTA: Once PRs start to exceed 10k lines of code, they seem to become slightly “safer.” I suspect this is because the extreme end of PR sizes includes refactors, which maybe start including less functionality change and therefore have a slightly lower chance of breaking. Alternatively, engineers may become progressively more reluctant to revert PRs after 10k lines because of emotional anchoring and merge conflicts.
[deleted]
Re: Claim: the ideal PR is 50 lines long
#9I have found that teams who allow larger PR sizes without complaining about it can get a lot done a lot faster. Which is to say having 50 line PRs are not ubiquitously ideal, but are somewhere on the spectrum of acceptable but come with definitive tradeoffs. Reviewing and merging 10x60 line PRs is, in my experience, more time intensive than reviewing one 600 line PR.
Most of the tests for and new functionality alone in any of our PRs well exceed 50 lines. Indeed we have 2-3x more test code than actual code. So should we be writing less tests? Merging the code first and later the tests? Merging the broken, codeless tests first and then the code?
It’s all ridiculous. Just make PRs that represent unit change, whatever the means to you. The unit functionality is independent of lines of code. Sometimes that is 12 lines, sometimes it’s 800. Yeah large PRs are harder to understand but that’s why you have tests. Also XXL PRs aren’t usually happening every day. If they are, you have a very productive team and maybe you should count yourself lucky.
Re: Claim: the ideal PR is 50 lines long
#10Forget about refactoring then
FTA: Once PRs start to exceed 10k lines of code, they seem to become slightly “safer.” I suspect this is because the extreme end of PR sizes includes refactors, which maybe start including less functionality change and therefore have a slightly lower chance of breaking. Alternatively, engineers may become progressively more reluctant to revert PRs after 10k lines because of emotional anchoring and merge conflicts.
I would speculate that PRs of > 10k line changes are more likely to be deletions of entire modules or assets that were dead code already.