|
|
|
|
|
by Smotko
2517 days ago
|
|
The article makes some good points. Here are my thoughts: > 1. Preliminary Checks +1 for this, but I would also add that you need to add as much formatting/linting into this step as possible. You shouldn't be pointing out formatting issues in your code review. > 2. Understanding This is super important! Pull requests that do not explain their purpose in the description should not be reviewed. The PR description is also really important when git bisect leads you to the PR when debugging a new bug. > 3. Usability Test A lot of debate regarding this, but I believe this is extremely important if not the most important part of your code review. For details read this section[0] > 4. Code Review A few good tips in the article, I'd just add that you should be polite and try not to waste people's time. [0] https://blog.codereview.chat/2019/06/27/code-reviews-and-you... |
|
> you need to add as much formatting/linting into [step 1] as possible
Yes, project formatting is a whole other topic but this assumes the project already has the team's linting/formatting rules baked into the build. As a reviewer, we are pointing out failing builds here, whatever they may be. If there are deeper issues/disagreements with the build tasks, that would be a separate issue.
Thanks for the link by the way.