Last fall, my development team grew from 1 person (me) to 3. Proper pull requests and code reviews all of a sudden became critically important to e...
For further actions, you may consider blocking this person and/or reporting abuse
This post is actually useful.
I had the opportunity of getting started to one team in code reviews.
I have two tips more to share them with everyone, which can helpful.
1 Priorities,
We had priorities with kind of issues such as security, performance, unreadable code, dry when it was needed, clean code we followed some patterns. This approach helped us so much.
2 Culture
Explain and make aware to your team the importance of this. Safety principle helps us feel safe when we make mistakes and learn from them. This principles must be permeated in all your team.
I'm glad you found it useful! Setting priorities for dealing with PRs is a great idea, and creating a culture where people feel comfortable sharing their code and receiving feedback is absolutely critical.
Great Article š
Thought I'd add, for step #5 use a pull-request checklist in every repository that encapsulates all those various items. That is a good list for a nodejs/react library. I'd customize for each repo dependent on languages. Lastly, make sure there is a section considering security and risk, again these should be repo dependent.
Thank you so much!
That is an excellent addition! And I'll admit, not something I had even thought about. But a checklist included in the repo is a great way to make sure you're not forgetting any of the steps. You could do the same to keep track of the required components of a pull request too.
put it in
.github/PULL_REQUEST_TEMPLATE.mdfor it to auto-populate. šGreat post!
I'm still trying to get better at #6 too. I keep thinking that I can review PR when I'm taking a break from my main task. How do you normally dedicate time to reviewing PR? any particular time or the whole day?
Thank you so much!
Setting aside time is definitely a struggle, and more than once I ended up working OT because I was trying to get my own work & reviews done at the same time. The best thing I've learned to do is treat reviews like their own dedicated task (because they are!), which means when I give time estimates for how long it will take me to deliver a project, I have to take into account how long I think reviews will take. So my time estimates are longer now than they used to be, but I'm fortunate that my manager is very understanding as to why that is.
I also will block off time on my calendar so I can't be booked for meetings or other tasks. The amount of time depends on the project. When we're building a website with a lot of interdependent parts, I will set aside larger chunks of time more frequently to do reviews so I'm not holding up the process. If our timeline is a little more flexible, I will do shorter periods of time (1 or 2 hours). I also prefer to do reviewing in the morning when my brain is still fresh, and I've found it puts me in a good place to start writing my own code when I'm finished because I'm more aware of good syntax, logic, etc.
I'm going to give that morning pr review routine a try. Thanks !
Good luck!
I've always believed in having my code reviewed by my team. If they can question that, it keeps me on my toes in all decisions, because I should be reviewing every important decision with the same rigour.
I think it's also important to make the comments clear and actionable. "This logic is vague" doesn't really tell me what to do to improve it. There's an opportunity to provide an example "would it be clearer like this ..." or suggest a straightforward enhancement "nested ternary expressions are hard to follow. This would be clearer if..."
I completely agree! Providing clear comments that are actionable is a must. We can't expect others to read our minds, and they obviously don't think it's vague/confusing/etc. because they wrote it and submitted it. I've found that I have started to write clearer code since starting reviews because I get called out on things like that, so now when I write it, I'm thinking about if others would be able to understand it from the get-go.
Wow, it's a really awesome article :) I like the part of everyone reviews especially by allowing 2 people with junior developers to look at your code to become better at answering your logic and write better code.
Thanks so much! It's been a really great practice. It gets them involved with the code review process from the perspective of a reviewer, which in turn leads to them writing better code on the first try (because they learn what to look for). Having to explain my choices and reconsider things I took for granted was an excellent bonus to implementing this rule, and it's made me change how I do a few things after realizing they could be done better.
If it is "them", get a team lead and HR involved. Don't resolve personnel issues on your own.
Of course, if there are larger issues/conflict with that particular teammate, you should absolutely escalate to higher ups and HR as appropriate. My perspective on this tip is meant to avoid generating negative feelings around code reviews by making it about the code and not the individual. Obviously if those feelings exist, there's a larger problem to be addressed first. :)
Sorry, I didn't mean to imply there's anything wrong with the tip at all. It's spot on. I meant only to add to it, to cover the uncommon case where it truly is a problem with the person.
Great read. Thank you
Thank you!
Thanks! Great post!
I just recently started transitioning to a "code reviewer" position so a lot of your pointers really ring true for me. I'm really grateful for #2!
Thank you! Good luck with your new position! It takes some time to figure out what works for you, but it's a skill that definitely improves with practice. Glad that some of my lessons learned could be helpful!
Excellent points! I just linked back to this article in the comments of one of my own on this topic...
10 Principles of a Good Code Review
Jason C. McDonald
Thanks so much! I remember reading your 10 principles back when I first was getting started doing code reviews and found your guidance extremely helpful in making sure I was spending my review time wisely. :)
Wow, it's awesome to know my article was helpful to you! I'm certainly saving yours for sharing with anyone new to the process.
Excellent post! Excellent! Thanks, @Jacque!
Thanks! Glad you enjoyed it. :)
Great post Jacque. I learned a lot here!
Thank you so much!