"Proactive" code reviews
Closed this issue · 5 comments
In the SD Mentor section:
- I am proactively reviewing other people's code so I can understand how best to help the team ship the best!
I take proactive to mean "I'm going out of my way to find code to review.". My problem is that taking time to do that takes time away from doing your actual work, being productive and shipping something. Pointing at other people's bad code is great fun, but does it actually help?
I think the intent is more like this:
- When I encounter anti-patterns and poor code, I share it and way(s) to write it better with the rest of the team in a blame free way. That way we can all learn how to deliver a higher quality, more maintainable product.
What do you think? Better? Worse? Too wordy? You have a different interpretation?
If you are using pull requests then I expect people to be proactive about spending some time reviewing code so that pull requests are reviewed and merged in a continuous flow.
It's obviously a balancing act because you don't want to take away from shipping features, but there should also be a recognition of the value of collaborative code review and the value that that provides in shipping the features too.
Besides, there will always be little bits of downtime between actively writing code that can be used to review PRs so if you do it right then it all works well.
@robdmoore A pull request is just a normal code review though. The fact that the process is integrated with the VCS doesn't make it anything special. And you're still passively waiting for the PR to be created, not proactively going to a dev before they create the PR and asking to see the code, right? :-)
Also, I didn't view this item as "I do code reviews with my team". I'd see that as a standard team practice and would expect it to come under the "I'm a supporter of my team" umbrella, not under the "I mentor others" umbrella.
Make sense?
Yep, I see where you are coming from now.
From that perspective it makes sense and I agree the part of it I was envisaging could come under supporting the team.
In which case I'm happy with your suggestion :)
Cool. Regardless, the confusion still exists in v2, so how about this?
- I use code reviews as an opportunity to teach and show others alternate, cleaner ways to implement functionality in an ego-less manner. That way the whole team together learns how to deliver a higher quality, more maintainable product.
Yeah that's better again