DEV Community

loading...

Discussion on: My code review checklist

Collapse
sigje profile image
Jennifer Davis

Great list Jenn! If I'm reviewing the code with the person who wrote the code, I like to ask "what is this supposed to do" before I dig in. Sometimes what it's supposed to do is different from the requirements as I understand them and it also helps me review that the tests are doing what they need to do.

Collapse
hermetnicolas profile image
Nicolas Hermet

I like to ask "what is this supposed to do"...

Isn't it the purpose of the pull request's description (assuming you're doing pull requests) ?

I'm deeply interested in your workflows of code review with PR, for we are really struggling to achieve that part in my team

Collapse
geekgalgroks profile image
Jenn Author

The Pull Request message and the requirements from the customer or business analyst can and sometimes do differ.

People forget to document clarifying questions, branches drift from the requirements (aka "I'll just fix this while I'm here"), and before you know it the PR is bigger or completely different than what was requested.

I like to keep things small so there is less drift and have an issue for each PR so clarifying questions are documented.

Thread Thread
sigje profile image
Jennifer Davis

exactly! So before I even look at code, I want to have a conversation with the PR author even for just a few minutes to understand the plan. Often folks in the commit message will describe the code rather than the plan. It's easy to get caught in that trap if you just look at the description and then read the code. "Oh it's supposed to increment this value until condition. Yes, it does increment this value until this condition."