Code reviewing is hard. The pull requests I reviewed were for the My Photohub project.
Code reviewing for the first time in my life made me reali...
For further actions, you may consider blocking this person and/or reporting abuse
I think the toughest part for me is phrasing my findings as polite as possible without it coming off as rude.
Just put all code review insights in the jira tool.. and give the owner to decide..that way egos can be avoided.
Yes that is difficult sometimes :)
tl;dr Code review, viewed as a kind of "lunch and learn" thing, has value.
I observe that people get too hung up on the code review as a thing to drive code quality. That's important for sure. But the other benefit of reviews is a way for a team to learn things -- whether that's coding techniques, patterns, new tricks, how their project works, whatever. So, rather then viewing code review as hard or a chore, consider it an opportunity to level up your skills
Very well said :)
I always pull my coworker’s branch locally to see better. 🤣
Good idea
Good suggestions, thanks!
I'd like to give a lot of credit to the way of approaching communication: positive feedback, attention to the developer's opinion and always think about what they may have thought while writing that line are the basis of a positive feedback cycle :)
Thank you :)
It may help to not think that you have to find issues; focus on understanding the code first. If you're having trouble understanding a piece of code, others reading it may encounter the same issue and you should probably ask the author to improve it even if you eventually figure it out. It may be worth trying a bit harder to understand the intent to be able to give better feedback (e.g. suggest a change that would have helped you figure it out 2x faster), but it's also valid to just state that you don't understand something (especially if you're short on time). The converse is also applicable: if a reviewer asks a question and it's not a trivial misunderstanding, try to answer the question by improving the code so the answer becomes trivial (or simpler). Do this even if they didn't request any changes. Think about how that code would be more readable: adding a comment, improving naming of function/param/var, defining a constant to make magic values self-documenting, restructuring code with additional helper function, improving testability.
Don't forget to look at the big picture (really, start with that). Good change descriptions start with self-contained one-line summary of what's being done and then additional paragraphs as needed to elaborate on the 'what' and also the 'why'. Don't need to write much about the 'how'--that's the change itself. If it's impossible to write a one-line summary, the change may be too big. Avoid useless/filler phrases (e.g. "fix bug", "refactor code").
Don't start nitpicking on formatting or minor details if the change is adding something that shouldn't exist in the first place (e.g. some redundant functionality), or if there are high level changes that need to be made which will probably result in a major rewrite of that piece of code.
thank you for your wonderful comment and insights :)
Loved the politeness of your comments. I must keep this in mind next time I'll be reviewing. 🙂
My personal approach to code reviews:
PR's should be prio 1, they should be done first, so people don't wait.
The reviewer should:
-- add their suggestions,
-- approve the PR immediately and automatically,
-- trust and leave it to the other person to process the suggestions as they see fit,
-- and don't chase it (don't double-check what they've done afterwards).
With the exception of important and critical issues, if any, of course.
:)
Great post 👍
Thank you so much for taking your time to read
nice article!
thanks!
Great Stuff!!
If anyone want's to follow each other on github here's my handle
github.com/dangolbeeker
I wonder whether it could be applied on data-related or AI / ML code
Great!
Great idea
Lovely 🤩
Thank youuu
Damn the 'Review 1' pull request is so long. I mean so many updates happened in that pull request itself.
Yes, true! A work in progress :)
This hellped me and my teammates a lot at work. The idea, is to use intents before each comment to help the PR owner to undersantd how important this change is.
conventionalcomments.org/