|
|
|
|
|
by awithrow
238 days ago
|
|
I think a lot of this stems from the code review tools themselves. Especially the "Don't just review the diff" mistake. Especially with GitHub defaulting to just showing a list of diffs and changed files. I'd find it much more useful if a review tool started with a class hierarchy or similar high level view to get a sense of:
1. What/how many classes changed?
2. What/how many methods have been added/removed/modified
3. What method signatures have changed
4. What changes are covered by new tests "Don't leave too many comments" i think can really be rethought of, don't review style and syntax. Leave that to the robots. If you're relying on other engineers to flag style problems and linting, you're just wasting everybody's time. Set up linting and style checkers and be done with it. |
|
You know you've progressed in their eyes every time they start bugging you about something new. You didn't suddenly become worse at something. Rather you got good enough on some higher priority thing that they knocked it off the list and replaced it with the next item in the backlog.
You should treat code reviews similarly. It's a journey, and we are in the middle. If you keep making the same comments on reviews, eventually they'll get addressed beforehand and you can point out something else.