Why do we do Code Reviews?
Code review is simply what it says on the tin. It is the opportunity for other developers to look at your cod...
For further actions, you may consider blocking this person and/or reporting abuse
Good points! I prefer giving concrete examples about names rather than use terms like "more descriptive". What's descriptive is subjective and I've been in situations where the PR author doesn't know what to change it to.
Tests is one thing I keep an eye out for. If there are none I don't approve it unless for good reasons 🙂
This is great advice and a reminder of the pull request reviewer's role: gatekeeper. Without their approval the code doesn't get merged. With their approval it can get merged. And everyone reviewing code should understand this positional power.
The review is the time of inviting others to help shoulder the burden of maintaining the "main" branch. It is a distinctly collaborative moment that both shapes and reflects the state of the code base. Review with a robust concern for the code quality and for the person offering the code.
Adding to the list:
is objectively more clear and concise than
But I too usually fall into the second while trying to be polite 😂
Except this time I declined a PR that used random generated numbers to IDENTIFY things 😅 The guy come and said that it worked in hims machine and that mathematics are exact science. I've lost 30min explaining why this is true unless you add randomness into the algorithm.
Sorry for the long comment, that's to say that CRs are fine but most of the time we need to group together with the juniors (and not so juniors) to make them truly understand what was wrong as mentoring process.
I always try to enforce KISS as base principle, then comes SOLID, guidelines and so.
Totally agree. The other day I got a comment on my MR about a trailing space.
Java developer, My reviewer usually points outs me over complicating things like:
Is that what they mean by KISS 😅?
if you got the comments such as "using a class for representing Money" several times, it might be a good idea to talk about it in another dev-jourfix or retro meeting. The goal is to clarify if your team can do a design decision together, if we apply an ValueObject Money instead of using BigDecimal generally. If yes, just put such thing into your team coding convention. Then everything will be easier in the future.
This is really sound advice everybody has a different way of doing a code review.
Great article!
Excellent points! Rather of using phrases like "more descriptive," I like to give real examples of names. What is descriptive is subjective, and I've been in instances when the PR writer is stumped. iogames-free.com
Very good points! On my team we take in consideration all this points! 🥰