Since so many days a Topic is in my mind. I would like to know, what is a code review for you?
Syntax and code quality check
Business logic check
Both
How much important is each topic?
How a usual code review looks like for you?
Please share your views and thoughts!
Top comments(4)
- LocationCalgary, Canada
- EducationBSc. Computing and Info Systems, Athabasca University
- WorkIT Architect at Olympia Financial Group Inc.
- Joined
PR review is a peer review. The reviewer ensures that the proposed diff addresses all requests in the story, and looks for code that doesn't belong to the story. They also desk-check it for any obvious errors. This is one end of the spectrum.
On the other end, I've also suffered through "team" code reviews, which can occasionally be useful, but mostly are uncomfortable experiences that don't encourage two-way communication and are an inefficient use of time.
The purpose of code reviews in our organization is as a general control. A developer cannot deploy without peer review, and this is enforced by Azure DevOps - you cannot publish to master without PR, and PR cannot complete without approval. While the idea of a team sharing session where ideas about coding are shared seems appealing, it doesn't seem to work that way in practice. Daily standups seem to be a more effective method of team communication.
- LocationBerlin, Germany
- EducationUniversity of Applied Sciences, Hof
- WorkI work with JVM
- Joined
I agree! But my question is more like, what is more important when Peer review is done? Business Logic (Here we assume UNIT Tests and Integrations Testing had already checked the logic with Amazing tests?) or Anti-Patterns, Design flows etc?
And what if the Reviewer how asks for changes has no knowledge about logic?
- LocationCalgary, Canada
- EducationBSc. Computing and Info Systems, Athabasca University
- WorkIT Architect at Olympia Financial Group Inc.
- Joined
I don't think there's any specific criteria that I check off when I review code, no more than when I help someone edit their business documents. Feedback from a code review might range from "please fix your indentation" to "I think this design doesn't work very well because...", or even "this is not the best library to use to do that". And of course "I think this feature is missing" or even "this code has a bug".
I guess if I had to pick something though, business logic is more important. While technical details like the ones I mention above are important, what I'm really looking for is whether the code solves the problem in the story. Stories rarely address technical details like what library to use or what application architecture to align with, so while I would comment on the library and architecture, the review is much more focused on the business logic.
- LocationBerlin, Germany
- EducationUniversity of Applied Sciences, Hof
- WorkI work with JVM
- Joined
Thanks a lot for your inputs! I completely agree!!
I always end up with some reviewers, with whom code indentation is more important than the business logic.
For further actions, you may consider blocking this person and/orreporting abuse
