4 ms·

Do I push back? If I care, yes. But I'm asking from the point of view of a team, not an individual on the team. How do you decide that something must be fixed?

sethammons 8y ago
As a team. Have a working agreement. What does good code and code reviews look like? What does it mean for a task to be ready to be started and what is your team's definition of done (level of testing, sign off of a code review, actually deployed, etc). These should be actual discussions and should probably be written down. When it comes to a particular code review comment, either it is reasonable or unreasonable. If you disagree, say so with an argument. Referring to a working agreement or standard is helpful. Either that is enough or it is not. In the case that it is not and the issue continues to be unresolved, grab a more senior person (hopefully on the same team) to weigh in. Maybe the comment needs to slide for now, but become a ticket in the backlog. If somehow this is still not resolved, bring in your manager.
seanwilson 8y ago
Just some ideas: - Have a code review wiki document you can refer to for clear cut cases of things you have to fix without debate (e.g. naming convention issues) and things that are more subjective and don't have to be fixed (e.g. there can be more than one return per function). - Code review suggestions should come with priority levels. Ignoring "nice to fix" problems shouldn't be a big deal if they're too time consuming and have little merit. This also helps draw attention to people that flag low merit issues as high priority. - Use linter tools and autoformaters to stop people wasting time with certain nitpicks. - Ongoing team discussions and training with concrete examples of which code review nitpicks aren't good uses of time might help. If you've got team members saying nitpicks are high priority and making no considerations about how much time certain fixes are going to take then it's a training and experience problem you're going to have to address. Can you give some examples of code review feedback you're not finding useful?

sethammons 8y ago

As a team. Have a working agreement. What does good code and code reviews look like? What does it mean for a task to be ready to be started and what is your team's definition of done (level of testing, sign off of a code review, actually deployed, etc). These should be actual discussions and should probably be written down. When it comes to a particular code review comment, either it is reasonable or unreasonable. If you disagree, say so with an argument. Referring to a working agreement or standard is helpful. Either that is enough or it is not. In the case that it is not and the issue continues to be unresolved, grab a more senior person (hopefully on the same team) to weigh in. Maybe the comment needs to slide for now, but become a ticket in the backlog. If somehow this is still not resolved, bring in your manager.

seanwilson 8y ago

Just some ideas: - Have a code review wiki document you can refer to for clear cut cases of things you have to fix without debate (e.g. naming convention issues) and things that are more subjective and don't have to be fixed (e.g. there can be more than one return per function). - Code review suggestions should come with priority levels. Ignoring "nice to fix" problems shouldn't be a big deal if they're too time consuming and have little merit. This also helps draw attention to people that flag low merit issues as high priority. - Use linter tools and autoformaters to stop people wasting time with certain nitpicks. - Ongoing team discussions and training with concrete examples of which code review nitpicks aren't good uses of time might help. If you've got team members saying nitpicks are high priority and making no considerations about how much time certain fixes are going to take then it's a training and experience problem you're going to have to address. Can you give some examples of code review feedback you're not finding useful?