Soft Skills Engineering - Episode 170: Code rage and code review etiquette

Episode Date: August 12, 2019

Vote for Soft Skills Engineering on the Hackernoon Noonies awards for best Dev Podcast! In this episode, Dave and Jamison answer these questions: How do I stop getting angry at other peopl...es’ code? Often when solving a complicated problem or implementing a feature, I have to modify or at least use systems designed by someone else. Often I find myself thinking ““Why did they do it like this??? This is so dumb!”” and literally getting mad in my chair. This happens no matter who wrote the code, and occasionally I discover that the author of the code was in fact Past Me. I know logically that everyone codes the best way they know at the time. So how do I avoid such a visceral reaction? Is this a common problem? Is this why many programmers seem to be Grumpy? My frustration often derails my focus and makes problems take longer to solve than they need to. What is the right etiquette for a code review for a pull request? I recently had an amazing code review. The reviewer pulled my branch, make a branch for changes he suggested and those changes all led to better and cleaner code. I felt the reviewer really tried to understand my design and test every suggestion before he wrote it. I felt that my code really got respect from the reviewer. However, a lot of my code reviews are just passive aggressive nitpicking like the comment formats are not right, the variable names aren’t clear enough. The worst was when I got a comment saying “this is already implemented” which after hours of figuring out what it meant was a different thing that would not work in my case. It seems like people have different ideas of what code reviews are and the etiquette and the expectations for it. As a reviewer and a reviewee, what should ideally happen in a code review process? Right now most code reviews are exhausting and infuriating experiences.

Transcript
Discussion (0)
Starting point is 00:00:00 It takes more than building elaborate monitoring dashboards that no one can understand except the creator to be a great software engineer. This is episode 170 of the Soft Skills Engineering podcast. I'm your host, Dave Smith. I'm your host, Jameson Dance. Soft Skills Engineering is a weekly advice show for software developers about non-technical topics. Building dashboards is the closest I feel to performance art that developers ever get.
Starting point is 00:00:33 Like you're creating this thing and it's about the experience of creating the thing. It's not about the thing being done when you're done because it'll just be like you had to be there to experience the full meaning, you know? You mean like no one except you will ever understand this dashboard fully or really appreciate it? Or the people that were gathered around you behind your computer. what would you you'd have like a keyboard case open for your fancy mechanical keyboard on the ground behind you for people to toss coins into and as you just like wrestle these prometheus
Starting point is 00:01:07 queries out of your brain onto the page and oh it's incredible okay you gotta build systems so that they export stats that when you query them in the right way draw pictures like your goal is not five nines of uptime your goal is put a smiley face in the latency graph that'd be amazing oh man that's i mean that's next level systems architecture yeah anyone can make something perfect that never goes down can you make it that it goes down exactly when you want to drop pictures that reminds that reminds me of get feedy have you heard of this no so get Feedy is when you craft a backdated Git commit timestamps such that your GitHub grid makes a picture. Yeah, I haven't seen that. Yeah. Yeah. But again, easy mode. Customer data isn't on the
Starting point is 00:02:03 line there. So it's not as meaningful performance. Yeah, I'm about to go on paternity leave starting tomorrow. So that's that's my attitude towards production uptime right now is like, sure sounds like a fun thing to play around with oh nice all right i think we have some patrons to thank today you want to do that for us i do yeah thank you to matthew voitovich the agile ventures charity ted nugent that's not the real one but we'll allow it still i guess crash bandicoot zach grannon this engineer goes up to 11x louis santos nick cantar taras karuk sean sunny tie Sonic the Hedgehog, Ivo Robotnik,
Starting point is 00:02:44 Mireille Rousseau, Chris Hogan, Chase Norton, and Stanley Tactical Radio. Those are the folks donating at the level where we shout them out every week. Thank you so much. And thank you to everyone else who is contributing.
Starting point is 00:02:53 If you would like to contribute, go to softskills.audio, click support us on Patreon, and donate at whatever level you'd like. You'll keep the lights on and you get an invite to join our wonderful Slack team, which is wonderful. It's great.
Starting point is 00:03:05 I enjoy participating in it, and you would too. Yep. And while we're on the subject of complete self, what's the word? self-promotion go check out the hacker noon website and go to the noonies which is a new award and we have been nominated for best dev podcast of 2019 currently tracking at position number three and we just need to make that position number one or if you want to do performance art
Starting point is 00:03:30 vote the other ones up move us to the bottom either way we're just happy to see movement yeah if you could try and arrange them so they spell like each letter of the of the podcast spells a funny word when they're in order or something that'd be cool too i'd accept that in lieu of first place right now it spells yes you have you have a blank canvas in other words yeah there's a lot of consonants there so those think of words with no vowels yes all right we have a comment from a listener would you like to read to us i would so this is from episode 168 we talked about a story of someone accidentally opening their own hr record while debugging a problem with this hr record system in front of the hr person and the embarrassment
Starting point is 00:04:19 that resulted so this question asker wrote in and said just wanted to say thanks for reading my question going into it it means a lot you're answering soft skills questions from other fields in it the problem actually resolved itself by the end of the week i was so awkward and apologetic that the hr person asked what is it that you think you saw and i responded i really don't know with the frowny smiley face emoji she then explained that it wasn't the hr file where complaints and private reviews and offhanded things that the business owners said about you were held it was actually just the file that held my old resume and current data like my position and vacation hours and such felt like after that she may have had more trust in me as i obviously
Starting point is 00:04:54 cannot bear the weight of an unclean conscience anyway i quit the job not too much later so So it all worked out. Oh, man. Great. Thanks for following up. I love hearing how things resolve. And it sounds like it worked out pretty well. And I believe that it is mostly thanks to our advice.
Starting point is 00:05:13 I will choose to believe that. If you've had a question answered and you would like to let us know how it went, whether it went great or absolutely terrible, we would love to hear about it. Go to softskills.audio and click ask a question and then tell us about your experience. We'd love to hear it. don't ask a question yeah okay do you want to read our first question yeah this comes from a listener named chris who says how do i stop getting angry at other people's code often when solving a complicated problem or implementing a feature i have to modify or at least use systems designed
Starting point is 00:05:46 by someone else often i find myself thinking why did they do it like this this is so dumb and literally getting mad in my chair this happens no matter who wrote the code and occasionally i think you can see this coming i discovered that the author of the code was in fact past me i know logically that everyone codes the best way they know at the time so how do i avoid such a visceral reaction is this a common problem is this why many programmers seem to be grumpy with a capital g my frustration often derails my focus and makes problems take longer to solve than they need to the question asker says everyone codes the best way they know at the time but that is not true if you're doing performance art like if you just read a book about tdd and you want to
Starting point is 00:06:33 try it out and you don't know how to do it which i may or may not have done that was not the best way i knew how to code but it sure was code that i wrote but at least in my defense i recognized while i was doing it that it was bad yeah i think chris has a problem here which is chris just cares too much you just care maybe not too much maybe he cares just the right amount i don't know i i gotta i gotta confess that i have also suffered from this like why why and most of the time i haven't let it surface i just you know keep it to myself hide it yeah behind this happy facade yeah but sometimes i have also you know let it slip that i was disappointed in someone's code which is not a
Starting point is 00:07:19 not really a great thing to do unless you're completely willing to take the full ownership of that and criticize your own self just as much so what do you just hide it what do you do is it a problem i'm trying to think it's actually been a few years since i've felt this way and i think what has happened is that i have moved on to problems that are so huge that i just literally don't have time to think about how well a little piece of code was written it just doesn't matter if this method is short well i think because it's it's going to be like such so many moving parts in this big system. Yeah, exactly. Like, yeah, sure, this thing could be improved. But we have huge architectural problems that need to be solved here. You know? Oh, so more like more like the
Starting point is 00:08:02 code quality is not the thing that will improve the system the most is what you're saying. I think so. And the other thing that has really changed in me over the last few years is I think a lot more about the end customer and their experience. And I'm like, look, it's working for them. I know it's not the best code but i take pride in the fact that it works you know i don't know that didn't that didn't sound very good just sounds like you would rage at past dave would rage at current dave a lot i guess i have a different reaction to it when it is code that exists already in the system versus when it's code that is currently being written if it's code that already exists in the system i i think i just lean on all of the bad code i've written in my life
Starting point is 00:08:45 and think about all the extenuating circumstances. And most of the time it's deadlines. There was just a thing that had to happen fast or sometimes this code was written early in the project when we knew the least about what this was going to do. So I think I have a little more patience with that. There's maybe, rage can be a powerful motivator for improvement.
Starting point is 00:09:07 You can rage out some performance art. If you feel really dissatisfied with the way something is, there might be productive ways you could channel that by improving it sophie alpert she used to manage the react core team and is a fantastic engineer and she talks about how one of the talents she has is that she's not scared to change code basically and there's especially in systems that are large that you might not understand well the tendency is to look at it and say this seems dumb but also i don't want to break everything so i'll just get mad at it and then move on but if you can channel that rage into improving things
Starting point is 00:09:43 around you then oh is it like is dragon ball z the one where they get really mad and their power level increases i've never watched a single episode i don't let's just go with yes because we neither have i okay our interpretation of dragon ball z is that it's a show about developers getting mad at code and using that anger to improve the quality of the code so be like goku i believe i think he's on dragon ball z and i think he really liked rust if i remember the episode right and write some rage-fueled rust code to improve the architecture you know that's you called out a different language and that's a really good point because you can use this rage to make completely unjustifiable conclusions about things like if we had written this in go
Starting point is 00:10:28 we wouldn't be having any of these architectural problems yeah there we wouldn't have all these parentheses because it's not lisp and this makes me angry that we have all these parens i've felt the same mellowing effect with age i wonder if it's a thing that happens often to folks as they work a longer period of time it's it's been beaten out of me i've done too many dumb things i've seen too many smart people do dumb things i've just seen too many products that work really that solve great customer problems despite the code being comprised mostly of spiders like it's just there's not there's not a text editor there's a box full spiders and you open them and they let them crawl around on you and then they go back in in a
Starting point is 00:11:11 slightly different configuration and that's how you make changes oh i said comprised instead of composed composed of oh i'm exposed as someone who doesn't use the right words oh man but it does persist forever in some people like linus torvalds is a code rager that's he his job is to know everything about linux and then anytime someone makes code he doesn't like to yell at them publicly and linux is great but i don't feel like that's most people are not linus torvalds most products are not linux so i don't know that that's a good way of relating to the day-to-day work of software engineering in general he's taken a lot of flack for that in the past year oh yeah yeah he certainly has and i think justifiably so like there there's a certain
Starting point is 00:11:56 subset of people that work in that environment and people i don't think most people like being the target of code rage. And so there are probably great developers that don't contribute to Linux because they don't want to be involved in that. You know, I think for me, there was actually a turning point, which was probably three or four years ago, maybe four or five years ago, even. I was responsible for rewriting some UI. And I got what I thought was really clever about how I was going to architect this system. I was going to separate the responsibilities in a certain way. Basically, I took all the logic concerned with navigation, put it in one module, and all the logic concerned with presentation, put it in another module.
Starting point is 00:12:39 And I'm like, great, this will be a nice, clean separation of concerns. You can mess with the navigation rules without having to worry about the presentation rules. And I built it that way. And then, of course, to really learn a few socket writing code, which I do, bring other people on the project a few months later, and they just struggled to understand the structure that I had created. And even I went back and I was like, oh man, this is terrible. This is really hard.
Starting point is 00:13:04 That separation was just bad. And I realized after all these years I've been writing code, I still can't take a basic architectural problem and separate the concerns out such that it's easy for other people to navigate. And boy, it was just really humbling at that moment for me. And I think that I just haven't really had CodeRage since. It was that failure that, I don't know, drained the passion from you. exactly yeah i mean that was before kubernetes existed
Starting point is 00:13:34 so to be fair you were working with imperfect tools then well you just restored all my passion dave's gonna come roaring out of this podcast and just rage on some code how do i avoid such a visceral reaction is this a common problem i i think to answer some of these questions specifically i think it is relatively common and a lot of this is because the the culture of software development i think encourages people to identify there's some amount of hubris involved that there is a right way to do things and you know it that's like a thing in the air i guess and and cool popular fashionable programmers like rants are a thing in software and people love a
Starting point is 00:14:24 good rant and part of the benefit of part of the nice thing about rants is it lets you pretend like you live in this world where someone knows the truth and and they can just angrily express it and and change things and i feel like so much of software is context and opinion based and we don't know anything absolutely about the right way to do stuff so you can certainly have opinions but there's just not a lot of concrete data out there and we don't we don't build software based on data for the most part. It's just opinions. And opinions make people rage a lot more than facts, I think. So I feel like that's part of why this is so common is everyone has this built up set of opinions about what is good. And one of those opinions is a meta opinion that my opinions
Starting point is 00:15:08 about what is good are good. True. But there are some cases where you could say, look, this is objectively more convoluted than it needs to be. And I've seen cases like this where it's like, okay, you pass in an argument to this function, but you're not actually using it. Like, why did you go to that trouble? And, you know, most of the time, these are oversights where someone was kind of iterating through their code, and they left some vestigial pieces around. But a lot of times, it's like, well, this is just the way it occurred to me to do it is, you know, I set a global variable over here. And then I use that global variable over here. And it's like, well, now you've got a convoluted, hard to understand system, you know, and I do believe that those
Starting point is 00:15:45 kinds of mistakes when there are better ways to do it they are worthy of fixing just not necessarily feeling all ragey about it yeah i like the focus that there because it sounds like you're leaning towards if there's a better way to do things that you know then it becomes a question of education and sharing knowledge and rage is not usually the best way to get people to change their behavior or learn things so if you're reviewing code or talking about architecture just saying this sucks is not helpful or motivating or how could you do talking about yeah yeah like well it was late and i was tired and it was due four weeks ago like that's how or or i don't know i'm a new engineer and i don't i don't know i'm doing the best i can but if you focus more on improving improving
Starting point is 00:16:36 things and improving knowledge on the team that's a productive way to channel it and then you can rage privately but i mean having brown bags is a thing people do you can you can pull up pieces of code and talk through it as a team although if you're going to be criticizing it it's best to do that with someone who is comfortable with criticism often i do that with my own code to point out bad stuff or like i'll do it with my boss's code because it's fine to criticize him no one's going to feel like i'm punching down you know yeah but but yeah if you turn your focus towards education and improving things, that might be the better productive way to channel your code rage instead of your Dragon Ball Z, like delete all the code that makes you mad and
Starting point is 00:17:15 write it good. I thought of another example from my own life that helped temper my rage a little bit. And, you know, I'm currently in a position where I'm a technical lead on a team. And the way my current company is set up is that that comes with a great deal of responsibility. And when you give direction to another team member, they take it pretty seriously. That's like part of our culture. So I do a lot of code reviews. And in one of these reviews, I remember telling the engineer, look, there's a simpler way to do what you're trying to do. Would you please go back and simplify this? And they went back and spent like the entire day trying to make the change that I suggested, because they really believed that I was right about this. But at the end of
Starting point is 00:17:53 the day, they came back and said, there are a few cases that I can't handle with your simplified version. And they described this to me. And I was like, Oh, you're totally right. I was just wrong about that. And I realized that, like, I spent five minutes reviewing this code called out what I thought was a simplification. But this other engineer had spent hours on this code knew all the edge cases and knew that it needed a complex requirement. But they were so willing to dutifully follow my direction that they spent all this time and I realized like, I need to be a lot more careful when I give direction like that, because sometimes we rage about code. And in fact, we just haven't put in the time to really understand the complexity that's actually
Starting point is 00:18:34 inherent in the problem. I've done that so many times too, where I pull up some code and rage at it and think, oh, this is so dumb, change it and do it this way. And then I don't say it in those terms. But if I try and suggest alternatives, I mean, sometimes I'm right. I'm not always wrong, but there are plenty of times where it turns out my quick, easy solution of why don't you just do thing x has a reasonable answer i mean i i agree that there's a spectrum here and there are some things that are obviously of no benefit and only produce maintenance burdens but yeah i guess the important thing is i i think i have developed enough communication skills that i can bring things up and identify the difference between times where it actually is important that it
Starting point is 00:19:15 is structured this way that seems less than ideal to me or times where we could simplify things and And that's safe to communicate in a way that we can talk about it openly. So I hide the rage. That's the answer. Nice. Just bottle it up. Hide the rage. Ask the question behind the rage.
Starting point is 00:19:30 Bottle it up and take it out of something productive like lifting weights. Exactly. Yep. Well, have we answered the question? I think so. Good luck with your rage. And we'd love to hear in five or six years if it's calmed down at all. All right.
Starting point is 00:19:43 I will read our next question. This is from an anonymous listener. What is the right etiquette for a code review for a pull request? I have recently had an amazing code review. The reviewer pulled my branch, made a branch for changes he suggested, and those changes all led to better and clearer code. I felt the reviewer really tried to understand my design and test every suggestion before he wrote it.
Starting point is 00:20:04 I felt that my code really got respect from the reviewer. However, a lot of my code reviews are just passive-aggressive nitpicking, like the comment formats are not right, the variable names aren't clear enough, etc. The worst was when I got a comment saying this is already implemented, which after hours of figuring out what it meant was a different thing that would not work in my case it seems like people have different ideas of what code reviews are and the etiquette and expectations for them as a reviewer and a reviewee what should ideally happen in the code review process right now most code reviews are exhausting and infuriating experiences wow this this is like
Starting point is 00:20:36 the flip side of the code rage coin yeah i love it it's almost like we chose these on purpose i know no almost like we motivated the question askers subliminally to submit questions at the same time about similar literally back to back good job listeners yeah thanks for picking that up yeah you got the message no really these did come in back to back completely outside of our influence this is a fantastic question yeah it's very convenient what's the right etiquette what do you think the right etiquette is well you know it's interesting like this question already has a description of the right etiquette built right into it it's beautiful like make a branch for the changes, tried to understand my design and test every suggestion before he wrote it. I mean,
Starting point is 00:21:16 that's fantastic. That is above and beyond, I think. Very few reviewers go to that level. Yeah. I worked for a while with a great guy named Murphy Randall, and he started doing this at Code Reviews at the place we worked together. And it was amazing. It was very helpful. And we found a lot of bugs and issues that would have slipped into production. We improved a lot of things, and it kind of spread to more people on the team the the downside is it takes so much more time than than like glancing through the code and saying oh you don't have one space in between your comment stuff and the start of the comment like fix that up please yep it's it's it's a task instead of a thing that you do kind of in between i don't know eating your sandwich like there there's
Starting point is 00:21:59 there's a huge difference in the amount of effort and dedication it takes to do pull request reviews to do code reviews like this versus the easy nitpicky ones and i do both kinds i definitely sometimes just kind of glance to the code do you ever feel like as a reviewer that sometimes when you come up with one or two little problems with the code that you can just send those off back to the author and then just walk away and be like well my work is done here even if you haven't read the whole thing yet oh yeah yeah the larger the code diff the i think easier it is to do that because it's hard to just comprehend more code all in your head at once and it's easier to say like oh i found something that has to be fixed so fix that and get back to me later about the rest
Starting point is 00:22:38 of it or whatever or or also like my work here is done you know this is me yep washing my hands of it my job is to find an error and i found the error yeah my job is to find fewer errors because of me and then and then move on yep it's one error better because and then i can delete that email notification about the the code review yeah exactly i have an app open on my computer right now that lists in the toolbar how many poll requests i've participated or should participate in and it's at 25 i'm gonna make that number go down to 24 with this one simple phrase looks good to me these four magical letters yep lgtm yeah so first of all that's awesome you got such a good code review it feels great and it's it's also i hope you said thank you to them because it's a
Starting point is 00:23:29 lot of work on their part and it would be nice to feel recognized for it sadly the passive aggressive nitpicking or or it might not even have to be passive aggressive it could just be kind of shallow quick glances yeah i think that's kind of the default at most places i've been yeah because like you don't feel direct ownership over this code right yeah and like you have to get your work done and time spent if you spend three hours on a code review that's three hours of time not spent on getting your stuff done and if you don't have an explicit culture saying this is a valuable use of your time then i think the the incentives are to get it out of the way instead of make use of it wouldn't it be cool if there was uh like almost a check box a list of check boxes that
Starting point is 00:24:15 reviewers had to check and the first one is just this looks like it will work you know like and and whether or not you are okay with the rest of the code or not you at least have to check that box as kind of a first step yeah like i believe this will not break everything kind of and it's like a it's like feedback back to the author to say yes functionally this looks correct and then now you you at least have that common ground you can build on for the rest of the review and you can say now everything we're talking about from here is about maintenance and you know nicety of the code and maybe performance and things like that right and performance art like you don't have enough ASCII diagrams. Not enough little slice of life comics in there. Exactly. I want
Starting point is 00:25:00 to see somebody making a bowl of soup in the comments in ASCII. Yeah. I mean, I think this is a problem that happens to every best practice that spreads through the industry, where it's widely believed now that code reviews are good and helpful. And I'm pretty sure when the idea of code reviews first started it had some pretty firm definitions and expectations and as it spread through larger and larger groups of people it's kind of diffused and watered itself down until now it's like someone should look at the code and do something before it gets merged in and goes to production but but like you said there's not a clear certainly not an industry-wide standard of what things you look for what order how long it should take how you prioritize it against other
Starting point is 00:25:49 tasks and even at most places i've worked that's been very implicit and you know i i would bet that the people who give the most detailed reviews also happen to be the same people who experience the code rage that i talked that we talked about in our last question and you think i think so because i think both of these things stem from a sense of ownership where you feel oh and just caring yeah you care deeply about about the code and i think there are a bunch of engineers who just it just doesn't it's not that they're bad engineers they just don't feel that sense of ownership like this code is my baby i think every friday at 4 45 p.m i am one of those engineers like if i had a pull request that i had to review and i didn't have a ton of context on it already
Starting point is 00:26:35 and it had to get reviewed then i it's a lot more tempting to say it looks fine then put in all the effort to get over the kind of knowledge gap of my current understanding to the understanding I need to review it successfully. I have two things I want to say here. One, as an implementer, you can make it easier to give good code reviews to your pull requests. You can do this by giving more context on what your change is. You've been living with it for a long time to work on it. You know the problem well enough to solve it. And most people reviewing it probably don't like their context switching away from something else. So they don't have it in their heads. So just explaining very clearly what the change is and why goes a long ways towards making it easy to
Starting point is 00:27:19 review versus if you just submit a thing that has like a short commit message and that's it or whatever. And then if you can talk about your reasoning for why you did certain things too, that's really helpful. If you want feedback on the design, if you say, here's what the design is and why it's a lot easier than if you just leave it implicit in the code. Yes. And if you can preempt questions like, why didn't you do this? Instead, write in the comments in your code or in the code review write-up. Also very good. Yeah. The other suggestion I had was around making these expectations explicit. The question asker specifically says, people have different ideas for the etiquette and expectations. And I think that's very true in most places I've worked.
Starting point is 00:28:02 And you could sit down as a team and say, hey, how should we do code reviews? What should the standard be how do we review them how much time should it take how much effort should we put in how should we communicate when we think things need to be changed like try and lay those things out explicitly and some tools even have built-in support for that github has pull request templates which which help you as the creator kind of fill out the required detail and could have little sections on like as a reviewer here's how you do here's how you do this thing like we have a a pull request template that includes a section for steps to test this. So that makes it clear to the creator that they should include steps to test it and to the reviewer that they should
Starting point is 00:28:42 follow those steps and test it and make sure it works. I really like you saying that you can make it explicit as a team what you expect from a reviewer. And I think you can take it to the next level and say, this is part of what it means to be a high performing valued engineer on our team and we will be looking at your code review contributions as a reviewer when we consider your performance review for you know pick your time series quarterly semi-annually annually whatever and it will be part of your compensation and we will compensate engineers who give the highest quality reviews better than those who give low quality reviews as judged by this fortnightly fortnightly comp reviews i would like that every paycheck is a negotiation opportunity
Starting point is 00:29:25 but like if you sometimes it goes down if you have very clear concrete expectations for what a good review looks like and your management is keyed into that i mean that's that's just a recipe for awesome and i've seen some fantastic reviews because of my company that is the case we we are judged on the quality of our reviews and when you're up for promotion they will be looked at and it's awesome because i feel that incentive when I'm writing a review and it makes me give a little bit of extra effort. It makes me go all the way to the bottom of the code changes and it makes me write more information. Like here's what I'm thinking and this is what's motivating me to think this. Anyway, I think it's great and I think more
Starting point is 00:30:05 companies should do it. There's a bunch of tooling you could put around this stuff, but I don't think the tooling would solve the lack of shared expectations. So that feels like the most important thing to tackle. And if you can get that in place, then all the tooling can help enforce those and communicate those as the team changes and people join and leave etc i think you could probably place some of the blame for this on the way our code review tools work though to where like a linear ordered view of i i don't know is it alphabetical whatever order your code review tool picks just reading alphabetically through the files that have changed is not not a good way to understand what's going on yeah wouldn't it be cool as an author if you
Starting point is 00:30:44 could kind of reposition things and say here's the high priority stuff here's kind of the boilerplate changes and you know that'd be really cool yeah it'd be great if there was some kind of semantic information or structural information too of uh sounds like a problem for someone who wants a phd somewhere but but if if if the code review tool understood the structure of your application it could say like here's the top level module that this change affects and and kind of here's where the interface changes and then here's where the changes flow through the application or things like that so i guess just build that and then your problems will be solved on the topic of tools though i feel so strongly that good automated tools that the team agrees on that can call out
Starting point is 00:31:27 anything that a computer can call out about a code change should be part of your review process so that a human doesn't have to waste any cycles on that at all oh yeah yeah if a computer can do it nitpicking comment formats is not a thing a human should do it because it it gives them it lets them check off in their mind like yep i did it i helped yes i pointed out where the comment format was wrong yes you should never you shouldn't you should never base if you ever type knit spacing like sorry you you just your tools are bad you got to fix that yeah yeah you want to raise the level of discourse above that kind of stuff variable naming i i mean that can be kind of a design choice and there are some linting tools in different languages that affect variable naming
Starting point is 00:32:10 but i still feel like that's sometimes a valid thing to discuss oh i think it's often very important thing to discuss i you know this is my bias i am a big naming guy i mean i really think it's important that variables communicate what they well i'm not going to go into details on how to name a variable but i think naming a variable is a lot more important than a lot of people give it credit for because other people have to read your code and if your variables are unclear or mislead then they'll have to spend those extra cycles understanding that and so i think code reviews are a great time to argue over variable naming yeah i i think it's so dangerous to get a variable name wrong that i don't even name them i would rather have just single letter
Starting point is 00:32:49 identifiers so you can't misinterpret what they do overall though there are some kind of first principles here which is as a reviewer you owe it to the author to be respectful and to put in as many animated gifs as possible i i still remember a code review i did once where i put in like 10 minutes to create a little animated gif that had some inside joke text on it or something and i don't remember what the change was at all but i hold fondly in my heart that memory of that time well spent you know oh yeah really just made the code better i actually do love that stuff i mean And when, you know, you have like a ship it gif that's like a boat, you know, animated. I don't know why I love that stuff, but I do.
Starting point is 00:33:35 Yeah, I think you're right about respect, though, especially when you're commenting on code quality. We always talk about how you're not your code and you shouldn't feel ownership of it directly. And people can criticize it without criticizing you. I say that's the collective we. The industry talks about that, but everyone still feels that. so comments that would not feel rude in other contexts can can feel a little more aggressive when it's someone commenting about the work that you did so i think it is worth putting in a little bit extra effort to say why why did you do it like this i think it might cause some problems
Starting point is 00:34:08 versus saying this is going to break everything yeah just like a little bit of extra politeness to kind of oil to grease those gears of communication is worth it and and making it a little more lighthearted is helpful too that changes the tone a lot from this inquisition or you being on trial to like working together with some some other good when i'm at the top of my code review responses i usually try to write the stuff that i saw in the code that was good like this is well organized or this seems to be highly performant good job thinking about that good job covering these edge cases and then i get into the things that need to be fixed and then do you end with some more compliments to make the crap sandwich
Starting point is 00:34:45 the code review crap sandwich otherwise it's just one piece of bread and then just a piece of poop you gotta have the other piece of bread open-faced crap sandwich and oh no it's the yeah exactly the open-faced crap it's a low-carb crap sandwich it's a lettuce wrap crap all right all right i think that means that's good i could i could literally talk for an hour about code review stuff um there's so much more that i really could say about it but i think we're out of time because we hit the crap sandwich we did yeah this is a great question and the answer might differ in terms of what is explicit what the expectations are for you but i think that's the key you got to make them explicit and follow them together so you don't have this drift between
Starting point is 00:35:32 what you expect and what happens and what other people expect yep all right what should people do if they want their own questions answered go to soft skills.audio and click ask a question and if you'd like to support the show you can click on support us on patreon any support of any amount greater than zero will get you access to our slack community which is super fun funny and fun and actually quite informative and insightful yeah a community full of over a hundred people now who are contributing it's been fantastic all right catch you next week

There aren't comments yet for this episode. Click on any sentence in the transcript to leave a comment.