4 ms·
I think I disagree with the emphasis on asking questions. Not because questions shouldn't be asked, but because they should be asked much earlier in the process
by allset_ 4y ago
I think I disagree with the emphasis on asking questions. Not because questions shouldn't be asked, but because they should be asked much earlier in the process than the PR stage. Every feature should at a minimum have a brief doc for the requirements and high level design. If you haven't read that and are reviewing the PR for anything other than style, you need to take a step back and go read that doc first.
In my opinion, the levels are:
1. Barely-existent review: The PR author is the SME, I'm only reviewing and LGTMing this because peer review is needed to submit the change. I might notice a typo or something along those lines, and will call out if they're being lazy and skipping tests.
2. Functionality-focused change: Does the code meet the objective and do the tests adequately cover the change?
3. In-depth review: Does the code address the problem in an efficient way that is not likely to cause the team future pain?