Soft Skills Engineering - Episode 101: Style In Code Reviews and How To Thank My Manager

Episode Date: March 17, 2018

Dave and Jamison answer these questions: A fellow developer submitted a pull request for me to review. The logic was totally fine, but the spacing drove me nuts. We use a linter to enforce some coding style but because this wasn’t a rule in the linter, I wasn’t sure if it was fair game to call him out on it. Was I being petty? I knew if this got into our code I would end up fixing it later myself. I told him I would approve the PR but thought that spacing should be more readable and consistent with the rest of the codebase. What is the proper etiquette here? Mention it and add the rules to the linter later? Don’t care about spacing if the code gets the job done? How do you express gratitude to your immediate supervisor? My immediate boss, who is lead engineer for our team, does an amazing job. Occasionally I get to peek into his world and see how much work he does. I am amazed at all he does for the team; shielding us from company politics, keeping us updated on relevant info, dealing really well with team drama and even makes time to contribute to code. How do I show gratitude besides building meaningful software? I recently read a paper on coding style and how it survives even through compilation and optimization!

It takes more than finely tuned Telios switches on your mechanical keyboard to be a great engineer. This is episode 101 of the Soft Skills Engineering Podcast. I'm your host, Jameson Dance. I'm your host, Dave Smith. If you didn't know, Telios are the connoisseur's mechanical keyboard switches. They only type Colmac. Yeah, I mean, congratulations to you, Dave, on reaching episode 101. It's a big milestone. yeah it's even bigger milestone than i think i was expecting by about one by one the the truth is the pressure of delivering an extravaganza for episode 100 was too great so we're just skipping

it and then we don't have to keep the expectations low and we will exceed them there really are two way to exceed expectations two ways to exceed expectations another alternative is to do like Reacted with their versioning. So they went from version 0.14 to version 15. So we could go from episode 99 to episode 9,999. Just really crank those numbers up. Welcome to our 10,000th episode. Find some bugs in podcast apps.

What happens when your episode number overflows a 32-bit signed-in? uh i'm pretty sure it can only be good for the listener base only good things right i was thinking we could go to some kind of semantic versioning scheme so where we have like a dotted triple for our episode numbers and then every time we give advice that contradicts advice we gave previously we have to we have to bump the major version number so every time which is effectively what we've been doing actually yeah that's true episode is a dot just imagine yeah imagine a dot zero dot zero after every episode number this is episode

101.0.0 um yeah we are consistently inconsistent we have some patrons to talk about do you want to thank them yes i do we discuss them i would like to talk about them behind their back uh so we have three weekly patrons we'd like to thank for donating at the level that gets them a shout out every episode that's dustin coates sean clayton and typescript tips also is that a computer or a person or a robot i think it's a bot network who's donating to our patreon was this a very prescient set of parents who are like you know in 30 years There's going to be a thing called TypeScript. And for a couple of years, people are going to talk about it a lot.

I'm going to capitalize on that. The whole rest of their life will be miserable because of their name. They'll have a brief shining moment in the sun. We'd also like to thank a new patron who joined at the one-time thanks level named Roman Pustilnikov. Thank you, Roman. That was a great job pronouncing that name, by the way. I have no idea how to pronounce it. And you carried it off with grace.

and poise or at least confidence yeah i think i would have broken down trying if not at your sight of at the sight of names that that look like they come from the different part of the country or different part of the world names that aren't your name yeah names i am an expert at pronouncing my own name most of the time i have spelled my name wrong before have you ever spelled it wrong oh yeah yeah i spelled it wrong before not for a while though it's been weeks yeah all right i will dive into it by reading our first question this is from a listener named sarah a fellow developer submitted a pull request for me to review the logic was totally fine but the spacing drove me nuts no spaces between the word else and the curly braces we use a linter to

enforce some coding style but because this wasn't a rule in the linter i wasn't sure if it was fair game to call him out on it was i being petty i knew that if this got into our code i would end up fixing it later myself i told him i would approve the pr but thought the spacing should be made more readable and consistent with the rest of the code base. What is the proper etiquette here? Mention it and add the rules to the linter later? Don't care about spacing if the code gets the job done? I think the only response that's appropriate is World War III. I mean, spaces. Well, you're saying the issue was there weren't tabs between the curly brackets and the else, right? Actually, what I was saying is that Jameson and I are 100% certified to talk about

this subject because we are space lawyers and it's true i think most people probably thought we were talking about outer space no we were talking about spacing in your code you know why why don't we okay tell me this tabs people why do you use tabs for indentation but not between the equal sign and the thing you're assigning sounds like you're a little inconsistent i'm gonna make a linting setup that does that i think so no spaces anywhere only tab yeah okay okay here's another thing i see people that align they want to align all their equal signs or align their their colons if they're they're like creating a an object or something there's basically all these symbols in a row they want to line up and they do they either have some

editor integration or some painstaking manual labor to put the right amount of spaces in If you just use tabs in between every token, then it's already aligned. It's fine. Okay. Dave, I think I've hit on a revelation here. Yeah, I mean, how hard could it be, right? I mean, surely different length words would never throw off that scheme. No, because the tab would be a variable length tab.

That would be up to your editor, right? and it's based on the previous lines tab placement yeah there's one tab stop per line they all end up just a presentation problem but the data model is sound listen i'm just a back-end developer but i know that the ui is easy oh man i've definitely never heard that um yeah this is a good question i had a thought while reading it yeah the thing that is unlikely to change anything is you telling this person i approve of this pull request but the spacing should be changed to be consistent because that means i approve yeah you can go off as soon as you say i approve

i stopped reading yeah so so there's no person who is responsible to go change it unless you just up and do it so if you want it changed you either need to say you should change it or you need to to assign that responsibility somewhere but just saying like i think this should be different without a person explicitly picking up that task will result in no one doing that that's the thing that happens all the time all the time all the time all the time and i believe it would happen in this case too the responsibility would just like diffuse into the ether but unless you have jameson dance on your team because if i recall correctly you are the king of cleaning up other people's spacing in code base and then submitting these like 10,000 line diffs yeah

clean cleaning up is a uh is one way to look at it so at my first job there were mixed tabs and spaces and this could not stand just how could i go on working when this existed so i just went through all the code and by hand just like typed a lot a lot a lot a lot and i think it took me several days and there were merge conflicts like crazy and but i merged it and then i'm pretty sure that whoever it was that put them in was just like doop-a-doop-a-doo and type on their editor again and like wiped it all out next time they touch those files but this has this has haunted me the desire to do this has haunted me throughout every job most recently i have been helped by the existence of tools that help enforce this so there's stuff like gofumped and golang prettier

in javascript that just like does it all for you you just apply it to a file it formats it you have no choice did you just say gofumt yeah that's how you say it wow i've never heard it said out loud i've heard from reputable golang people reputable gophers i guess yeah yeah that that's how you say it gofumt that is that is classic i mean these people clearly come from a c background right they totally do i could talk for a long time about the the culture of the language and and the people that are attracted to it it's really interesting and i like it you and your malik and your sprint f no malik and go thankfully no no i'm just going i'm just hearkening back to the sea that brought them here oh yeah they definitely they definitely

like knowing well i'll shut up um we just like opened fandora's box and peeked inside just a little yeah the point is i have done this a lot and i've done it in ways that provided no benefit to the team and that no one bought into and i've done it ways that provided benefit to the team i've also been on teams where um they set up linting that conflicted with my personal viewpoint and i i you said world war three dave i went to war this was the maddest i've ever been about a technical decision at work what like by far oh my gosh um and and i had what i believe were some valid reasons but a lot of it was just just emotion about like this thing is wrong and bad ah so there's just something about style that creates uh intense emotions in people that care a

lot about it and i'm one of those people and they're also people that don't care at all and just are like why why do you care how many spaces are between this curly brace i do have a friend who said that he likes working in code bases where the style is inconsistent because it's like little signatures from different people you can tell like oh this person did this part because they use the gcc style or whatever and uh or what is it i don't know gnu c i don't know whatever that style is knr style yeah that's what it is that's probably what i mean so you're like it's almost like him it's like walking through an old town and you're like oh i'm in the latin quarter yeah yeah exactly it does not bother him one bit that that curly braces and spacing and all that

stuff is inconsistent well i mean think about all the missed opportunities he had to feel rage i feel like his experience is incomplete yeah yeah he probably worked a lot less hard because he was less fueled by yeah passion exactly he probably slacked off from a whole bunch i think there's potential for discussions about formatting and spacing to nerd snipe certain people and i am one of those people for sure um i am i am easily distracted from my day from my real work to like space stuff out correctly or at least consistently totally snipable yeah i have evolved so i don't care what the style is but i still care a lot about it being consistent and i i think oh go ahead dave no i'm just i'm just sitting here thinking about you that's all

i got nothing to say i hope you have stuff to say i could talk about feelings about spacing forever but i need you to interrupt me i need to backspace this conversation a little bit i i think i think you and i are very different on this subject i just do not care and i think there may have been a brief point in my life where i cared but i can't remember it i remember maybe feeling that way but i just don't i am so happy to adapt to the style that's going on in the repo I'm working in, I'm like, whatever it is, I will pattern match it to the T, you will not be able to distinguish my code from anyone else's by the spacing. And I just go with the flow. And actually, I'll tell you what else. I'm actually kind of hypersensitive about it. About 10 years

ago or so I was working in C++. And one of my developers who is a very humble, very quiet, unassuming, not in your face guy. He made a very subtle comment about how I put extra spaces inside the parentheses of my if expressions and my function calls you know what i'm saying like i would do a paren and then a space and then the parameter list and then a close and then a space and then a close you're a monster that's what you're saying i actually don't i don't think i see this this style very often but i did it every all the time i did it for a couple of years and he made this one comment and as if to point out that that was kind of silly and i looked over at i'm like oh no and he just instantly like shut down he's like oh no no nothing never mind forget

forget i said that he's seen the wars he saw the wars in my eyes and that man is a veteran and uh he just kind of shut down the style wars this is this is his style he's very uh un unobtrusive i guess you could say as a person and yeah i i immediately changed i was like oh my gosh he doesn't like that style that spacing style so i just immediately switched and all the code i wrote going forward i got rid of those spaces and so i think i'm just very adaptable and very um what's the word influenceable gullible i don't know what what the word is so i think there's a couple different facets here one is like caring a lot about a specific style and the other is caring about consistency what i gather from this question is is it might be

about consistency because she specifically mentions the spacing should be the spacing should be more readable and consistent with the rest of the code base and and i think i think my friend who doesn't care about consistency is an outlier i think there is an argument you could make just from culture that that most teams prefer style to be consistent yeah i think there's a very good pseudo-scientific junk science argument you could make about that yeah it also increases productivity it it eliminates these arguments if you have it consistent and if you have tooling to automate it okay that's and that's the thing i like wait more about hold on does it eliminate the arguments or does it just defrag them and come consolidate them all to one gigantic argument at

the beginning of a project which is like do we use this thing or not like well arguing over what the final linter rules are going to be for like three days and then you never discuss it again yeah so there there are opinionated linters and they're unopinionated ones and i think unopinionated style linters are are a breeding ground for more discontent but the ones that just like put your code in get your code out can't do nothing about it those are my favorite like gofumt like gofumt and prettier um they i guess there there are a couple minor exceptions to configuration with prettier but the uh the the the joy for me there is not that i love the style that prettier formats my code in it's that i just like smash my hands on the keyboard and type

nonsense and i i spend zero percent of my brain worrying about making something look a particular style and then my editor just does it for me or my some script runs and does it for me and that's even more valuable than having a consistent code base it's consistent but it also happens automatically yeah i agree i agree i i think the point is that if if you have a a an agreed upon set of styles there are tools in several languages that will either warn about those styles being violated or even change it to fix those styles automatically and if there's a style that is followed in the rest of the code that isn't in those tools then put it in the tools and then you don't have this problem of like having to be the the person nagging someone about their style

and and blocking their important functionality change or feeling like no one's going to change it so you have to go back and clean up after them or just it's just a bad bad place so i feel like an effective use of your time i totally agree with that and i feel like our tools are really weak in this regard because what happens is we all have our editors and even some advanced editors and linters like you're describing they will present code to you in the style you prefer and then they will commit the code in the style that the team has agreed upon so that the data model like the underlying data store has the code in a canonical format but the problem is a lot of these tools we use for code review they are not aware of these preferences right so we have a

situation where we have a reviewer reading code in a style she doesn't prefer and we and if our tools were better they could just say look i understand that the way we store the code and the way we present the code are two different things and i know each person's preference and wouldn't that be great i mean then you just would completely bypass these problems because the computer doesn't care it's only about the user who's reading the code in their editor or in their review tool yeah that's interesting i haven't thought about just completely eliminating all styling except in in an editor that's kind of the argument you hear for tabs right is like i'm gonna figure out how indented things are yeah like what's your tab stop size yeah

it seems like there sometimes is some meaning communicated in in white space like you group things together they're probably related uh everything on one line is probably like hopefully that's one idea you can kind of glance at the line and abstract it instead of looking at each individual token in the line and if you took all that away and left that purely up to the individual editor configurations you might lose some of that yeah that's fair i don't know but either way i think the the solution here is put it in the linter hopefully your linter can fix automatically if not just fix it i think it's fine to care about code spacing it's it's a hard discussion to say i would like to modify our style but it's an easier discussion to say like hey we

have this style we should enforce it yeah that's true that's true and also at one of the things that i see that differentiates effective senior engineers from less effective senior engineers is their ability to recognize things like this that they don't need to be spending their brain power on you know you have a limited amount of time in your day limited amount of effort you can put into your job and if you're spending your time talking about white spacing in every code review, that is time that could be spent on higher value activities. And so effective engineers will force these discussions to happen outside of the code review so that when the code review time comes, it's not part of the discussion. Because that is overhead, right? That will bog down the

review, take you away from the important points that you need to be reviewing, and that a human brain can review effectively while bogging down the conversation on stuff that a computer could have just done for you without any thought required. And it comes right down to it as a scalability problem. Like you can't scale as a senior engineer. And if you're needed to review all these things, but you're having conversations, which by the way, these are all asynchronous conversations, right? You're leaving comments here and there. And then that takes 20 minutes for the other person to get back to you, right? Or an hour or even a day if they're in a different time zone, right? Like these things just add tremendous overhead for very, very little value.

So I would say that if you encounter something like this in a code review, ignore it. And then offline later after the code review is finished set up a system to block this kind of conversation from ever happening again for anyone on your team yeah i like that suggestion i think we agree we agree even though we disagree on uh what is it what did we disagree on i forgot it was like you care a lot about the way that the code looks and i don't care i think is what it comes down to it i mean i care how it looks to a certain extent but i don't care about spacing in certain ways like extra spaces after curly braces and things like that i think we agree more than you think we do because the thing i most care about is that it's consistent and then i do care somewhat about my

own personal style but i have like gladly abandoned that in favor of tools that format stuff see i don't even think i care that much about the consistency i mean i care a little but not not that much i i value not having to think about it so now there we agree and and a tool that makes it consistent means I don't have to think about it ever. All right. Awesome. We read the next question. Yes. This comes from a listener named Satya who says, how do you express gratitude to your immediate supervisor? My immediate boss, who is a lead engineer for our team does an amazing job. Occasionally I get to peek into his world and see how much work he does. I was, I am amazed at all he does for the team, shielding us from company politics, keeping us updated on relevant

info, dealing really well with team drama, and even makes time to contribute to code. How do I show gratitude besides building meaningful software top top suggestion um put this podcast episode on a floppy disk some kind of physical media slide it onto his desk just leave it there you mean put the first minute of this episode on a floppy disk well not the first minute the first minute of this question perfect it's like a instead of passive aggressive it's like passive friendly passive nice i don't know it's passive effective passive so passive effective so it'll be like a treasure hunt while your boss tries to find a floppy disk drive reader a floppy disk what know you call those yeah drive trying to find a drive yeah and then figuring out how to adapt a

parallel port connector to a usbc yeah that would help that would make them feel very very appreciated we had similar ideas when we were writing down comments mine was to change all the get commit attributions for your work to their name just make it look like they got all your work done yep and mine was to every time you fix a jira bug put your manager's name on it so your manager gets credit it's like a unique marketing opportunity where people below you pass up some of the money they earn to you and you pass up some of the money that you earn to the people above you forming kind of a triangle or perhaps a pyramid no oh yeah that's a better way to think of it yeah because it's in three it's in three dimensions really if you think about it so

there's it's like a pyramid uh layout yeah i think a pyramid scheme of jira ticket closing is appropriate in this case and 10 of all jira tickets you close go to your boss your upline yeah um i think it's fine to just tell them to their face if you have one-on-ones with this person or if not if you just send them an email or send them a note or just say hey i really appreciate the work you're doing that that makes everyone feel good i think workplaces in general could usually benefit from more people saying things that they appreciate and like about each other that's true i have never seen a workplace where someone said you know we just have too many people saying thank you to each other we need to cut that back

a little bit. Yeah. We often focus so much on performance and often that turns into focus on things where you can improve that, um, just kind of appreciating and calling out all the things that people do well falls by the wayside. Yeah. We have continuous delivery. We have continuous improvement. Why don't we have continuous gratitude, continuous compliments. I love it. yeah i i think you could just tell them that would make them feel great and it's not hard is it though is it hard to walk up to someone and say thank you it's a little awkward i guess maybe it is yeah i guess maybe it's hard if you're just like i'm just gonna go there and say thank you yeah hopefully you have one-on-ones that feels like a good opportunity for it or you could

just say thank you when um if they do in the moment right they uh i don't know they shield you from company politics you find out about that somehow so you just say hey i heard i heard you did this thing to prevent us from having to deal with this pain thank you thank you i appreciate that i mean that's so easy to do i think i think when you have a specific instance it's 10 times easier to walk up to someone and tell them thank you um because you can just tell them what it meant to you that they did x i mean that specific thing it's great whereas if you just walk up to someone and just say hey i just want to let you know i think you're really great and thank you they're kind of like what that was out of the blue yeah yeah it could feel a little weird then

i have a little trick that i do um pretty often probably do this a few times a year when i sense that someone is doing great work and i want to tell them thank you for that i will often send a brief email to their boss and i will say hey i just want to let you know that so and so has done a really great job i think they're a great engineer here's a few things they did that i was very impressed with and i just want to let you know that i appreciate having them on the team and i'm really thankful that i get to work with them and then you send that email to the boss and then you forward it to the employee also just to let them know hey i sent this to your boss people love it i've done that i probably do that a couple of times a year and i genuinely mean this

like i would never do this just to i don't know earn points as someone but it makes people feel so good and it makes people it makes me feel really good when people do that for me it's very very thankful because not only is it just a gratitude thing but it's a gratitude it's a it's thanking them in a way that will benefit them you know by by helping their boss shape a positive image yeah i really like that i think i've done that in a less structured way just like recently a couple people helped me out on a thing and while while updating other people on the status of it i made sure to call them out and say like and these people did some extra work to make this happen so some of it could be giving credit to people too but but i haven't done that explicitly

where i've just said like who's their boss they've been doing great stuff i'll tell them i really like that and it it doesn't cost you anything either right like it's it's not like there's a zero sum game of that's right you'll you'll thank them and now you lost something you yeah you used up all the goodwill i i i think this would lead to people having more trust and respect for each other in general if you create a culture where where people appreciate the work everyone else does and that's motivating in a much different way than like gotta shape up or i'm not gonna get the rating i want or not gonna get that promotion or this person's gonna be mad at me yeah i i really like that you asked this question though

because yeah i don't think i've ever explicitly said thank you to my boss for doing nice things for me i've been very influenced at work by how by the kind of stuff my boss has done whether i they were doing things that felt like they helped me a lot or less so but i haven't really taken time to explicitly say thank you i like that that makes a lot of sense totally good in fact i think it would be good if i had like a recurring calendar reminder probably on a monthly basis that says hey think of someone you appreciated in the last few weeks and send them a note to say thanks yeah i think that'd be a great mechanism to help remind you to to do this because frankly the world could use more of it and consistency is important here because it's the kind of thing that can fall by

the wayside yeah um satya is asking about their manager i think it i mean it could apply to anybody too and it also applies going the other way managers thanking their uh their their team members i feel like that might happen a little bit more often just because there's there's often an interaction where a manager might ask someone to do something they go do it and then they say thank you for doing that thing but i'm sure it doesn't happen too much still right yeah yeah probably not too much can there really be a too much yeah that's what i mean like yeah no there cannot what about okay would there are there any like really out of bounds weird things you could do to thank your manager like buying them a car good or bad no there are plenty of bad things

yeah bad that's what i wanted to say like where are the boundaries here like should you get your manager should you buy them a basket of candy should you you know are there gifts that are inappropriate obviously there are but like yeah you know when where do you draw the line i mean sending a nice note to their boss is one thing but that's obviously within bounds but i'm sure there's some stuff that maybe would be out of bounds i don't know why would you do more than that i don't know maybe you just feel like the little email is like the director of a play gets roses at the end of it or something exactly the end of a sprint by your boss a fruit basket yes i don't know would that be inappropriate i think i don't think it'd be inappropriate i i

would be like why i don't i guess thank you but why did you i didn't need it can i expect this fruit basket every month or after every sprint yeah i think the bar is so low that just saying thank you is is great okay well we'll circle back in six months when the world is full of gratitude and overly thankful and we'll decide once we've solved that we'll we'll raise the expectations a little bit i don't know don't touch them don't like going for a hug yeah yeah a long extended handshake staring deep into their eyes keep it keep it profesh keep the guideline all right well i like this question i feel like thinking about this question has made me a better human being yeah me too i want to say to you satya thank you thank you for asking this

question thank you you know dave thank you for putting it in here because you selected it i did and i'm gonna send a note to your boss to say thank you for uh all of satya's good work we don't know who satya is but we will hire a private eye pay them and thank them nothing says thank you like hiring a private eye to track you down in your personal life yep yeah um okay i think we've answer this question yeah borderline into the creepy land and yeah that means the show is over backed swiftly out what can people do if they want their own questions answered dave go to our website at softskills.audio and click ask a question then as long as you're there go to twitter and follow us at softskillseng where we tweet about stuff we'd love to have you

all right thank you for listening we will catch you next week