3 ms·
> Write small diffs (pull requests) (this one is actually good advice) I work at Meta, my opinions are my own, and I'm speaking to this point in general, not s
by dehrmann 4y ago
> Write small diffs (pull requests) (this one is actually good advice)
I work at Meta, my opinions are my own, and I'm speaking to this point in general, not specific to my time at Meta.
Small, incremental changes are great because they're easier to review, easier to verify, and safer to roll out. What's important during the review is seeing the whole picture. It's possible for changes that are locally sane to produce something globally goofy. I've also seen changes for larger projects with multiple reviewers miss an important piece because each reviewer assumed it was a change the other person looked at.
- vsareto 4y agoI think it comes down to team/company preference. I would really prefer a culture that allowed (at least) 2-8 hours for a larger code review of features. That way you can block time out for it and digest things. A company culture has to allow that. People get reluctant to review the large PRs because they know they have to other work to do. A bunch of small model/service/repository PRs seems tedious to me. If I want the consider the whole picture, I also have to go back to each PR.
- dnissley 4y agoWhat I've really taken to heart is telling a story of my changes using a stack of diffs. A random one line change to some sketchy function is a whole diff so that you can call it out as something to be aware of, etc.