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.
Jenn is a self taught web developer who specializes in usability and accessibility. She is easily spotted at conferences by her bright lipstick and various code dresses and t-shirts.
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.
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."
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.
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.
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
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.
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."