13 tips to make your code reviewer NOT hate you

Key Takeaways

our friend John Cricut is back with 13 tips to make your code reviewer love you review your own code first I think this is probably the most Junior mistake ever and for the record I am not a senior engineer I'm a mid-level if you can even call me that but when you first learned a code you pretty much never like read your own code unless like there's a bug once you get it working you pretty much never look at it again and that's kind of the opposite of what you do in the real world I didn't make this mistake too many times where you don't really look at your code and maybe you leave like an extra print statement or just something is messed up like formatting and things like that thankfully I didn't make this mistake a lot but I've seen other people make it a lot I've worked with people on N code iio where I've had them do like really really basic tasks for me and it's so obvious that they don't even look at their own work before sending it to me and it's just kind of hilarious I worked with somebody who was working with me for over 6 months and every single time almost every time they would have very very trivial mistake so please look at your own work before you send it to somebody else cuz it's kind of embarrassing if you leave silly mistakes in there write a clear change list description most of this entire list is not really a skill issue it's not like a skill diff it really is in my opinion just about putting in effort I saw at Google there were people with experience and without experience who would not really have a descriptive message for the commits and for the changes when you're a reviewer if you don't have any context about those changes it makes it harder to review and so either one the reviewer just doesn't care they're going to rubber stamp that poll request or whatever or two they're going to probably ask you a bunch of follow-up questions and then that adds more back and forth that kind of wastes more time so if you just give a good description from the start which is really not that difficult to do in my opinion it just saves time it just makes people like you more automate the easy stuff I don't know exactly what this is talking about maybe like styling and linting and things like that so your reviewer doesn't have to catch those like small mistakes answer questions with the code itself maybe that just means have readable code make the function names pretty descriptive variable names descriptive so that it's pretty obvious what the code is doing if it isn't obvious then you should add comments but at the same time I think an easy mistake for junior people to make is to have too many comments and that's kind of a waste of time if the code is descriptive don't add a comment saying what exactly a function does unless it's like required in like J doc or something like based on your linting rules sometimes for public functions that are exported you have to have a JS do comment at least that's what the guidelines were when I was at Google narrow Le scope changes this one at Google was mandatory like if you had a really really big change your reviewer was just allowed to tell you to go f yourself not literally but they would tell you hey I'm not reviewing this break it down into smaller ones and it's Justified because a big change is just harder to review and there's more of a chance that a bug or issue might slip through I've seen it happen where somebody had a CL that's what they call them my Google but it was over a th000 lines maybe almost 2,000 lines of code and it took him a long time to get that change to send out to review and then the reviewer was like nope break it down into at least like four different changes maybe even more unfortunately it's such a big change even to refactor it into separate like commits is going to take you all day at least to do that so it wasted a lot of time unfortunately separate functional and nonfunctional changes that might be like refactoring I think might be an example if you have a change that's refactoring a lot of the code you should probably just not change any of the actual logic just refactor it and don't actually change the function of what that code is doing and then if you actually wanted to change something make it clear like you don't want to have a commit and it kind of goes into the previous one you don't want a commit that's doing too many things so I think that would probably make sense break up large change lists I guess I kind of got this confused with number five that's what I thought he meant but I guess with number five he probably meant more like the functional changes that you're making don't make like five different functional changes in the same poll request respond graciously to critiques I think that's good and it goes both ways of course you might have a reviewer that's giving you a hard time usually reviewers aren't doing that it can feel like that it can feel personal when somebody critiques you for a lot of people to be honest I think this is something I didn't really have a problem with I actually liked getting criticism I liked getting feedback because that helps you grow like that's really valuable when somebody more senior than you tells you hey you just made a mistake here I find that super super valuable not just when it came to code reviews but even with YouTube comments like if somebody points out a technical mistake that I make in a system design video a leak code video even which sometimes I make a mistake it's super super helpful like you know a lot of people pay other people for advice so when you get free advice you have to really appreciate that and I think that kind of mindset helps me to not take things personally be patient when your code Reviewer is wrong it's always an awkward situation if somebody like gave you a piece of feedback and I think it actually happened to me at least once or twice where somebody says hey do this or maybe you're supposed to do this differently once I actually like believed them and I said okay then I'll do it this way is this what you meant and then they're like oh sorry I actually didn't have the full context on that if you disagree with your reviewer I think it's perfectly fine to explain your reasoning any type of critique or response should always be justified you can't just say like to a junior developer or even when you're arguing back to a more senior developer you can't just say like well I feel that this is better you have to give at least some kind of technical reason why in the case of somebody gives you a 2,000 line poll request you can't just tell them I don't feel like reviewing it you have to tell them it's more likely to have bugs if you give me a really big one but if you make it smaller I'll be able to read it better even if the truth is you just don't feel like doing it that's still a technical reason so try not to get too emotional I guess in code reviews try to keep it objective and have reasoning for everything communicate your responses explicitly I think that goes with what I just talked about I guess actually another point that comment how I was working with somebody who would send me some poll requests and I had to review them if somebody leaves a comment telling you to do X Y or Z and then you respond with the changes you 100% should address that comment you have to leave a comment saying okay this issue has been addressed because otherwise then you have to go and look at the code and of course the reviewer should look at the code but if you come back to a PO request in the future you don't really have the history like whether you're going to make a change and address a comment or you're not going to address that comment you have to leave that history there probably within the poll request you can't just make changes and then not have the comments in there that's just something I guess that annoys me but I mean if you guys disagree with that definitely let me know artfully solicit missing information I guess I can't 100% interpret this one but it could mean like if there's a confusing part in your code you can sometimes proactively leave a comment on that code that's already been resolved and say like okay by the way I know I might get a question on this piece of code so here I'm explaining it within this comment and things of that nature award all ties to your reviewer I I think that one's not super important like if there's like an opinion difference like a thing could be done one way or the other the reviewer is kind of in control they can kind of dictate the pace they can block you from getting it merged so if there is a tie and it's really not that important if it's nitpicky as the person trying to get the code through it's your code and you probably want to get it pushed through as quickly as possible assuming it's correct just for the fact of speed you probably should just defer to your reviewer cuz it doesn't really matter anyway minimize lag between rounds of review you can't always control whether your Reviewer is going to pick up and review things I Google it was pretty quick usually within a day sometimes though for readability reviews where somebody outside of your team is going to be reviewing a change I would have cases where it would take multiple days the best thing you can do is just like send them a ping and remind them cuz sometimes they forget or sometimes they're just lazy if you do get a response from them though it's probably good for you to address their comments sooner rather than later so hopefully they can finish reviewing it rather than waiting again you'll have to wait multiple days in that case but I think overall this was a really really good post this really speaks to kind of the social nature of working in teams and writing software I think this is the type of stuff nobody's really going to teach you in school and to be honest you guys can let me know if you had a different experience but nobody ever taught me most of these things like Google has code review guidelines and some of this stuff was actually in there like break up big changes into small changes but the social nature of it nobody will teach you that nobody taught me that so this type of post is really important I think if you want you can go and add me on LinkedIn there should be a link in the description