Excellent advice. One thing this doesn’t mention that I ran into a few times in practice is the fact that PRs don’t show what hasn’t changed. Now this makes sense, but in the context of a refactor that fixes a naming inconsistency, for example, the reviewer will have to make a checkout of the code (or somehow browse it) and grep around for missed changes. Or in the case of a big refactor in functionality you might not see that some old functions were kept around that are now effectively dead code. It always leaves me with an eerie feeling of “what else might we miss during code review?”
Excellent advice. One thing this doesn’t mention that I ran into a few times in practice is the fact that PRs don’t show what hasn’t changed. Now this makes sense, but in the context of a refactor that fixes a naming inconsistency, for example, the reviewer will have to make a checkout of the code (or somehow browse it) and grep around for missed changes. Or in the case of a big refactor in functionality you might not see that some old functions were kept around that are now effectively dead code. It always leaves me with an eerie feeling of “what else might we miss during code review?”