DEV Community

Discussion on: How do you code review?

Collapse
 
artis3n profile image
Ari Kalfus • Edited

Outsource all style issues to a linter that runs in CI. You don't need to spend brain power worrying about bracket placement.

Aside from any checks of the complexity and maintainability of the code, the PR itself should have some context as to what problem it is solving. I recently rejected a one-line change PR because it was setting an environment variable and there was no context in the ticket or PR about why this was being set or what problem it was solving.

Each PR needs to strive for greater maintainability of the code. They need to tell a story about what is being solved. Without exception, any PR that isn't solving a problem is a waste of time. "Updating README" vs. "Updating README to make it more clear how upstream libraries interact with this API." Now there's a story, there is context for the code changes.