Erik Rose Constructive Code Review PyCon 2017

so next up it's going to be Eric gross and he's going to tell us about constructive code reviews please have it welcome him warmly welcome thanks so much for filling this big room this is going to be about how to run code reviews so you build the best possible product but also build people up and build yourself up while doing it so code review is to have this connotation of sitting in a stale conference room pointing at stale code with stale long sticks modern code review is still fail but generally asynchronous and web-based and so we can weave all these advantages with it we can take you can take your time and do it whenever you want but then there are still some disadvantages like we use we lose rather the ability to build rapport face to face I'm going to offer some suggestions on how to overcome this and other disadvantages and get the most out of a very valuable practice I've been through hundreds of code reviews both giving and receiving I've worked my whole career almost in open source of various kinds where reviews are not we never stop arguing about code you may recognize my name from such open source libraries as parsimonious or itertools blessings we're always arguing about code on those things I've been Mozilla for seven years where everything we do is open source and goes through code review so I've also spent a lot of time mentoring interns junior devs showing them how to review and be reviewed the goals of a good review are three build a good product and this is a code review kind of fills the role of ensuring the thing works beyond what our tools and our tests can tell us by rubbing a second brain over it rubbing a second brain over it also ensures that the code is understandable to people who didn't write it second we build excellent programmers we spread knowledge of the codebase like well Yuri implemented this but there's a routine over here in the utils module that does it we spread raw competence like way did you know about generator expressions we can make this lazy and not concretize the whole list all at once and we spread the love of the craft of programming I mean really what an amazing privilege it is to program what could be more creative then not only authoring a thing like an author of human text does but having it run with a life of its own Wow I want to spread that joy to as many people as possible and then third in code review you build yourself whether it's working with volunteer contribs users clients or paying customers it's easy for their demands to build up and make you feel like you don't control your own time like you're just guilty all the time I should really be working harder and harder and I'll give you some tools to avoid burning out on that now of course the opposite of these good things can happen you can waste time nitpicking you can tear people down or you can burn yourself out through failure to set and enforce personal boundaries and this is a real problem because coding is creative work and creative work is powered by enthusiasm this is particularly true of open source even when it comes with a paycheck we are an intrinsically motivated crowd especially the kinds of people who come to PyCon right and kind of open source people I make my living doing this but I don't get paid to program I see it as I get paid so I can spend my time programming rather than getting money some other way now the creative spark is a fragile thing and it's all too easy to snuff it out in people if you're lucky and this is the best case they'll quit if you're unlucky they will hang around and they'll write bad code so how do we preserve or even fan writer this creative spark we do it through equal parts truth and kindness you'll see that everything I say today is going to be motivated by one or the other piece because after all the thing has to run on a machine made of metal can't argue with reality but on the other hand we're all these squishy emotional beings and we need to live sometimes so let's start with some truth the clarity of explanation as you're looking through the code what did you find this comment is self descriptive a better comment would have actually explained what happened okay well this could happen temporarily and it moves to the next thing and it goes through this routine it gets caught up in gears and we ended up with the use after free and we could have a security hole now because I think is good but not sufficient we also need to be durable our comments need to be durable over time and over code based change so if I were to have a link in there and point off to something on the master branch and we did this review a month later the master branch has moved man there's those lines of code they're all over the place now people have added people have deleted if you're going to link link to a specific commit we also have to be durable over what time they're going to be looking at it I mean so that month has gone by again if my comment said hey you remember when we talked about yesterday yeah do that not helpful remember last day we talked about in the chatroom maybe also not helpful not everybody keeps chat logs see what you said in the chatroom when we talked about that thing and the free after whatever and then would be good in fact heck if somebody comes back to me a month after I made a comment and said hey what did you mean by that so most review systems are web-based for better or worse so why not at least use the affordances that the web gives you most of these systems have code form of formatting for code snippets so learn it and you can use that same thing to paste in a repple sessions pipe that has a lovely interactive interpreter you can paste stuff in there you can say well I ran your procedure out of number three and it exploded and look see you can have links after all this is a big hypertext links to specific commits remember links to relevant documentation and these kinds of more equities were specific again but language neutral comments are really nice because for some people English is not a first language some people don't know each other well and can't can't interpret your hand-wavy comments or or take your sarcasm some people are new to the project and give the extra context if these pointers provide and some people just haven't had their morning coffee now sometimes in the course of a review comments go back comments go forth and you find yourself talking past each other you're just not getting your point across in those cases you can up the game to a higher bandwidth medium like a video call or an audio call or heaven forbid pick up the telephone if they still know what those are and don't just do it any old time interrupt them out of their engineering day and agree at a time in advance and make your call sometimes your time zones are different and whatever you do afterwards be sure to write down the results because some people are not in the conversation and it gets worse as time passes and no one remembers the conversation clarity of explanation has a little brother clarity of expectation be clear not only about what you found but what you're asking the person to do about it and how much you care not such a great comment yet again but if we say all right I like to do this but it doesn't need to hold things up it's not a showstopper that's that's great in fact I biased toward merging things in quickly and not continuing to nitpick and nitpick and nitpick because after all if it's better than it was why not let everybody enjoy that better you can almost put off the polish unless it's a security hole or it's a typo they take you no longer to file than to fix one of the wonderful things do we have we have the wonderful github reviews which are a relatively new feature and they lets you be more kind of enumerated lis explicit about I'm just saying or do I actually require this change in order for me to in good conscience approve this bias toward I'm just saying it's my opinion and then at last when I'm done reviewing a patch I like to be even more explicit about my expectations by saying that's all I see with those changes I think this is ready to merge that lets them know I'm done let me know I'm done if I come back and look at it later and it ends with a little note of encouragement a little bit of kindness once we have those two great truth bringing clarity's in place we can move on to some kindness now none of us likes to be criticized so code review is this really kind of unnatural act at asking for critical feedback it's a really bizarre sort of thing and our human tendency when when critiqued is to react in you scan somebody's amygdala you find it reacts in the same way is that you had thrown like a Blagh at their head it's like a physical attack oh don't don't compete my coat I love my coat so even when you don't take the time to really softly wordsmith things it's easy for people to take offense and things to get out of control and you have bad faith going back and forth and so since nobody does have time to wordsmith every little comment here are some quick hacks that impart tack without requiring you to be shakespeare the simplest of these is the humble question mark thank you dr. evil there's no point returning path results when there's more than one term a little Stern maybe a little arrogant verses can you remind me of some use cases for returning path results when there's more than one term yeah a little a little more inviting a little more open and this is an actual review that I did with my my intern a couple of summers ago and in this case there was a little bonus in that let me save face because there was totally used cases for that they're really prominent a second bonus is oftentimes the question mark will activate the Socratic method imagine a review that would say hey what if we had another thread going here the route to this variable oh says the other person maybe we could have a race condition and then maybe yes I see but then and then what if they were sensitive data and oh okay I'll go fix that so now maybe you've got your point across maybe you've taught somebody something and you've done it together you we in this so if you say you a lot it feels like an attack a lot if you do it this way you're going to mess it up you idiot versus if we are doing it we're building something together you know if we divide 8 by 4 we have to in a very textbook a kind of way and if that's too textbook II for you you can always keep it about the code and say well this this casting will behave thus tack tack number 3 is a good old compliment of course there's the old trick if you're going to say if you have to say something critical say something complementary before and afterward compliment sandwich that helps cushion the blow the best time for compliments I find is early to set the tone and I like to compliment the spirit of the patch before I look at I like to say oh wow look at this be sure you implemented I'm really excited to get it I know yours is going to be excited let me take a look and I'm able to make this this positive comment because I haven't seen the code yet a favorite of mine is uh well thank you so much for refactoring that scary mess that I wrote last week and compliments can of course go both ways hey thanks for catching that reviewer good catch oh I didn't know about that oh you saved a big bug good job tactic number four is humor a couple of cute kitties always improve somebody's day Mozilla's reviews in particular seem to be awash in animated gifs lately and it keeps things light hearted and fun and this should be fun right we get to be creative we get to make stuff we need to sit in a comfy chair and press buttons all day in an air-conditioned room it's a good gig so question marks you we this compliments and humor these are some tactics we can use to soften our critiques now aside from being harsh let us see some other anti patterns that reviews can fall into have you ever submitted a thousand line patch have you ever gotten this in response the only solution to this is to make these long patches somehow less dreadful because somebody's looked at this and said fine fine the best contributors to my projects the ones that I end up hanging handing the reins over to eventually submit patches that are a pleasure to review even if they're long and the way they do this these champion committers is their pull request comment you're the one that encompasses the entire patch is a couple of paragraphs long it's like the abstract to an academic paper it says here's the problem I approached I thought about this alternative in this alternative and I chose this alternative because of this okay great that's totally easy for me you took another five minutes and save me reverse engineering the state of your entire brain and getting it wrong so good use of time team time and they have long commit messages to these long commits I'm so more in Turtles project as the you know eighter tools these things so performance matters and some of my champion committers have put benchmark results right there in the commit message like wait look it's 5% faster than whoa okay great I can just hit merge reckon I can run your example and say yes there commit sir are small and logical they do one thing at first I fix the formatting and then I added this routine I needed and then I implemented this user-facing feature using it and finally they the comment code where they couldn't make it clear in a different way they have great doc strings on everything because nothing's ever obvious they think it's obvious it's totally not obvious and they named things well routines and objects and attributes and such when you get a thousand line paths without these things do not by any means feel guilty about asking for them it is going to take less time I promise for that person to clean it up and it would take for you again to try to get inside their head magically and figure it out and of course the opposite is also true when you submit a patch do this with a checklist now sometimes you're in the heat of a good hack right and you're not really proceeding block-by-block from small structured commits to small structured commits that's just not reality sometimes you need to make a large sketch of portion of the program just just a flush an idea out or see if something is even plausible and in those cases you can use a tool like get X this is a Mac one there get K does similar things it's gtk base 2 runs anywhere and I'm sure there are others and these let you and get anyhow stage let me not use jargon they let you break up changes after the fact into commits these lines go in this commit this line goes and that commit these lines go in that one this is the thing called interactive staging if you want to read about it more and there is a command-line interface to this but it is horrible and should be avoided because it's this maniacal 20 questions thing it's hard to back out of it very modal just get the clicky thing you'll be much better off the other thing these tools help with is they show your dips you should totally read your dips every time before you submit before you come into even I mean who would give somebody a non-trivial piece of writing without ever proofreading it that's that's silly don't do that caught so many print statements and debugger break points with these things it's not even funny and because we spend so much of our time looking at discs consequently it's hugely worth it to get a diff viewer that you like I find the unified diff red and green Christmas tree kind of distant I find that kind of hard to grok after it gets past a certain complexity I have to use a part of my lexical brain to say red means deleted and green means added and that's brain power I don't have anymore to think what the code means so I like something like a file merge on the Mac and this is a free thing it comes with Xcode I love it because of the Bezier curves it's a little thing but it has an outsized effect on me like this line balloons into those lines and these collapse into those and I don't need to use any lexical power at all all this old next-step stuff is great from 1996 and a whole different operating system it'll different ire directory trees for you it wires nicely into your version control system you can do merge conflict resolution with it drag that bottom little handle up and you get a third pane where you can do freeform editing if the back-and-forth isn't enough great tool it's not the only one like this go shopping and find something that fits your brain so those are long patches little patch did this ever happen to your one-line patch yeah people just felt like they had to say something didn't they so nitpicks by definition they're small things but they can add up to a big effect it's a continual pitter patter of discouragement it makes it hard to move fast and discourage you from making small fixes I've even left a team because of a nitpicking culture in part and because it's a cultural problem it needs a cultural solution all this stuff comes down to interpersonal stuff nitpicking often comes out of people groping for status trying to justify their existence because they're insecure or I found something wrong with that big fancy coders code so now I'm a big fancy coder like really and when do you when you've in the second grade so a solution to this is self confidence and I'm sorry to say that because self confidence is tricky it's almost a misnomer because you can't give yourself self confidence you can't go and sit in the room and like gin it up but fortunately it's easy to give other people self confidence and you should practice this in your team all the time and I say team loosely whoever you're working for is if you're on the same team you're not working against each other be generous be frequent with your compliments give credit where credit is due often and in public right always give your compliments in public and give your critiques in private and code remix that really hard but nobody's reading your review of that involvement who wants to read extra reviews right what else we got ya remember what it costs to sit there in that chair per hour remember what they're paying you what your client is paying you and throttle accordingly right view is this really worth loan per hour is it with that per hour for them to address it and if you do all the stuff consistently you'll see half of these nitpicks just evaporates the other half of nitpicks have a semi mechanical solution do you have a style guide for your project you should have a style guide and this lets you have every argument about style only once these are great places to start if you're not familiar with a Python enhance our proposals these peps Google pep 8 talks about whitespace and spelling and other such glorious things pep 2 5 7 builds on that talks about docstrings the phrasing the formatting there's the Polka style guides I like Armin Ron occur and his his buddies put that together and it kind of fills in the holes where that leaves off and then finally we have Spinks formatting I use for argument function arguments which the rest of them don't talk about and it's a nice terse syntax and it brings with it the advantage that you can run the Sphinx documentation tool over your code afterwards and get nice talks out why not now once you've chosen a style guide you can automate the style checks using a linter who knows that linters everybody knows about linters love winters ok a popular linter for python is flake eight it's fine I'll be careful because overzealous winters hurt the understandability of your code code is for humans I would pause it and if you end up with these little winter turds everywhere and they get longer - those are short ones but I mean they get kind of crazy and if you have them all over the place you need to turn that stuff down you have to read past it all the time you think oh comments valuable notes for a computer and your tools should serve you not the other way around with a healthy culture and mechanical support turns with a you know rigged so that it only enforces what everybody agrees on all the time you should be able to spend your review time on things that matter not little nib pics here's another thing that stops up a lot of projects have you ever made a change to a function and then someone comes along and says oh can you mix the rest of that module while you're at it it's been a mess for years we have a joke at Mozilla which is kind of a haha only serious that if you touch a particularly icky bit of code you now own it forever try to keep that in the realm of haha or there becomes a disincentive for fixing broken windows right and the idea after all is for the codebase to be continually getting better not being perfect so yes point out problems if it's not going to distract but don't block an unrelated patch on them or distract the submitter by you know jumping into the middle of a relevant thing with the near OLAP relevancy and as I said my rubric for merging to master is just better right merging and if you pull down the slides afterwards these are actually links up to the c2 Weeki the original wards wiki particularly good entry points any time wasted reading words wiki is time not wasted it is energizing take your patches turned around quickly and the opposite to get them turned around very slowly so even if you aren't able to be comprehensive right now you can say I see this and this at a glance looks pretty good otherwise I'll give you a full review in a couple of weeks or I don't know what I'm gonna be able to this somebody else can feel - feel free to jump in and finish it don't feel like you married a patch just because you said something on it also because stuff falls out of working memory very fast as expensive to load back in for the patch submitter it's important to address it while they still have still remember what the heck they were thinking the other time it's really important to turn around quickly is if you have to say no to somebody if you have to say this is philosophically against our project or you need to rethink your approach or there's a big thing going to land and just obliterate this in a minute it's best to do that quickly so that they don't waste their time waiting around for Santa Claus or building more castles on a sandy foundation so so far I've been giving very specific cases but now I want to talk about the more general emotional issues behind them for instance insecurity is probably my least favorite human emotion that it's called caused Wars and it's just a kind of fear that's all it is what are we fearing when we're insecure we're fearing at the time that people will find out that we're incompetent to some degree almost everybody feels this doesn't really go away impostor syndrome that's what this is I read a little long-form tweet earlier in the week by the author Neil Gaiman he was at some kind of awards banquet for Wolfram various things and he end up in the back of the room and he hit it off of the other guy who was there because both of their first names were Neil and the other guy was was complaining to him he said I hate coming to these things I this room full of creative people who made all these wonderful things and all I ever did was go where I was sent it turned out the guy was Neil Armstrong so if it's good enough for the first man the moon it's good enough for me right realize nobody's judging you because they're worried about you judging them they're all wrapped up in their own thing thinking about you at all but they're not thinking about you at all okay you can just go ahead and do the best job you know how to do worrying about nothing but your own concept of good work likewise don't take it too hard when somebody does find something to critique in your code sometimes it's just having a fresh set of eyes we all have that sometimes they genuinely know something you didn't in which case you'll know it afterwards so really you win either way nobody should lose in code review not a game the final defense against insecurity is to notice when you're feeling a strong emotion notice it and name it I'm feeling anger I'm feeling disappointment I'm feeling fear why am I feeling that way is the next question and sometimes you will not be able to answer that question and it'll be because you skipped breakfast or because your dog is sick in which case it's pretty easy to dismiss these things sometimes I don't know I'm feeling that good sometimes it's for because you actually do believe something that isn't true like boy it really matters then I can spell this variable the way that I want or it really matters that I win on my team for some value of win and you can refute those little cognitive behavioral therapy you can even say it's not true and kind of let go with that stuff a bit but emotions once you feel them take some time to clear have you ever had a dream that really hit you hard and you're really sad in the dream really afraid in the dream or and it kind of stuck with you the whole day long and all day you felt yourself easily falling back into sadness or whatever the emotion was there's this chemical persistence that happens with emotions where it's it these chemical sponges these neurotransmitters get out of the gate and they attach to things and they literally take about 10 minutes to BRE up taken and filed away so when you find yourself feeling a strong emotion and and you don't want to speak out of it because I almost make bad decisions when I'm doing that it's a great time for lunch that's what it means it means it's time for lunch it's time for a walk jump up and down get the blood flowing get those neurotransmitters back in the box and you won't regret what you say speaking of emotions one way I often feel is short on time anybody else nobody feels that way let's get this one so there will always be more to review than can be reviewed there all's Lemoore to do than can be done broadly in life in general for a period of years I would be viscerally repulsed by the pile of full requests stacking up on my open source projects as they would stack up I feel even worse and become even less likely to look at them say it stack up even higher and just goes around and around and around and finally I had to come to peace with the fact there's always going to be more to do than I can do in fact if we were to list out all the constructive things that we have the option of doing at any one time it would be a couple of hundred hours of work even if you were to take a couple of weeks off you would never catch up I mean you maybe you get some of those things done but they keep on coming in as of two days ago my list of things I could be doing was a hundred and sixty-one items long I keep a list and so we only have three options open to us we can lower our standards and probably still not get everything done we can give up on sleep or we can choose just to do the most important things so then the only question is what are the most important things how do we determine that six years ago my coworker Chris wandered by my desk with a stack of these getting things done books and I said hey free book and I took it and it sat on my shelf for six years because I thought well I don't need some business starchie guy talking to me about how to run my life and making it overcomplicated strict system I'm a creative free spirit man so I spent years feeling guilty instead as those thoughts magda me you should be doing this over here not that oh well you should be doing this over here Oh No that's why it's not my life it's really obnoxious so today though I have almost no open-source guilt I've been more responsive as many contributor can tell you and I've been an inbox zero for a year both personally and professionally and it's easy crazypants I can't tell you many times I try before that so I made a one slide version of this book so it looks like a lot but it's just a couple of branches it's only a couple decisions in this tree now so when you're doing one thing and that nagging thought comes in maybe you should be doing this other thing instead of having it go around and around in your head and distract you you write it down write an index card throw in a bucket write a text file throw in a folder whatever you want to collect them in collect them in an inbox of some kind then go back to what you were doing and you'll feel you'll feel at peace because you know it's not going to get forgotten you don't rehearse it your brain knows that once a day go through your bucket and then push those each index card down through the rest of these this decision tree so let's say oh I should take a vacation gear up this summer summers coming up okay write that down and later when we go and see it we say okay well let's see is it actionable yeah I could totally start working on that I can start researching for example so we go down that left-hand path what's the next action maybe go TripAdvisor and see what people like all right well that finish it well no I still have to book travel and do everything else so if that's actually a project and a project is a multi-step to do and so what we do is we write going to give you plan a vacation to Europe at the top of the index card and then once we do that action and it's empty we know well is the vacation your plant no okay so I know to add a new thing it's a prompting to add a new thing later on let's take another example I have a staircase in my house and I have a filing cabinet at the top of the staircase which is a stupid place for a filing cabinet that's where it is and so what actually happens is the stairs become the filing cabinet and I would walk through my house I work from home I walk through the house and I see the stairs and I see stuff on the stairs and I said person I'm so futile I can't even get the stuff off the stairs how am I going to get any work done and so hmm there's stuff on the stairs hmm well it goes into the Inbox is it actionable yeah okay what does the action put the stuff off the stairs and file it will it take less than two minutes heck yeah just do it right now don't even write on the index cards at two minutes yeah just do it and the stairs have been empty for the most part I'm not perfect the stairs have mostly been empty and when I pass the empty stairs I think I'm a useful person I can get stuff off the stairs maybe I can even do something else and it tends a cascades into the next thing and you get rolling so when you're like stuck in your work do something stupid wash the dishes and see if it bleeds into the next thing more than often more likely than not it will if you take nothing else from this slide is too starchy for you just take the two-minute rule it is the best part of the book it will handle 80% of your email by itself it will ass wage 80% of your guilt and and let's do the other example if the booty else okay so we got a sentence so we have the only APR to review right so if this PR and in fashionable yeah so so what'll I do with it well I can delegate it to somebody else I don't do this much if you're a manager you do it more if you're married you can tell you stuff to each other you want to take the carpet oil change or not okay I'll do it calendar it can go on your calendar but only if it is something that has to happen at that one time it can happen to no other time I have a haircut appointment from noon to one on Wednesday that's good if I don't get it then the appointments up and it'll take me weeks to get another one otherwise it goes onto the next list and that's the one with 130 things on those are all the things that I've decided to do I decided they're worthy of doing and I can choose to do at any one time I go through that every day and I make kind of a little today list like to do today so and the reason this whole thing works is by logarithms which I'm getting a little head of myself first let me show you the other the other branch up top let's say we have a pull request to review that it's not actionable because it's a second-round review Sally made some code I made some comments Sally is going to go address those comments and then we'll do the sec around review but Sally's not done yet so you know in came the github notification email that she's made some changes but she's not done yet so that actionable no well is it something we put in reference like my personal wiki or electronic filing cabinet for later no not really mmm that's I'm happy let's see if that gets better is it something we should defer until another date yeah quite possibly maybe I have my liberal computer system say I bother me about that in a week and until then I don't want to see us or maybe it goes into a someday list if I don't even know when I want to be notified about it and I think this is a someday maybe list like someday maybe I'd like to go see the pyramids that I'm not committed to it so I'll just kind of throw it in that bin and then what I will do is I will review it every week that's the other piece of glue that holds this whole system together the weekly review every week you look at your Sunday list you pick things out of it and promote them to the next list you look at the next list you say I don't really want to do that after all it's no longer important anymore you push it back into the sunday list or you look at things on the sunday list and you say I don't know what I was thinking there that no longer applies it goes into the trash and so what I was thinking about logarithms logarithms and we know about trees right as programmers the reason this whole thing works is it's order log n every week we look at the some day list and we you know take some parts of that big list and put on the next list every day we look at the next list which is smaller and take parts of it put on the today list now that the day list is only you know 10 or 14 things long and we can linearly scan that all day long without going out of our minds in fact if you have and I have maybe 200 items in here but if you have a thousand if you have really you'd have to have about ten thousand to a hundred thousand to justify this but you can add another even at another level I actually have a double some day stuff that I don't review every week I look at it once a month like I don't need to decide about the pyramids every three week and that's really the whole book that's really it and as a result I don't feel guilty anymore I don't forget stuff anymore and let me give you a little bored about inbox zero and why it's in to be it inbox zero and I don't think people talk enough about why it's important and it's really important because in boxes make a crappy UI for a to-do list right if you let your if you let your email box be a to-do list it's a to-do list that somebody else that's the priorities on without thinking it's always the latest thing it's at the topics that the most important probably not because the email box is a nexus of lots of different things coming together you end up doing this repeated linear scanning through it all the time does that apply no where's what is that eliminated is that appointment expound on your calendar where's that reference material I needed was it in the mailbox because not in the reference database where it should be what should I do next let me check my to-do list and look at my patch queue let me look at the email box and you end up scanning through each of these emails more than once why should you ever read an email more than once unless you enjoy reading it you shouldn't that stuff goes right into the inbox I triage my email maybe once a day and I do the same stuff with it points go on the calendar so to do things go on the next list and the reference material goes into reference and frankly most of it goes into the trash aka archive oh no the other thing of course is if you look at your email box most of these things you can only see one or two dozen emails at the most at a time and once something is falling behind below the fold no matter how many flags you've applied to it you're never going to see that again and the whole thing adds up to no longer dreading looking at your mailbox don't you hate that you have something you're supposed to do like my mailbox was my papers on the stairs every time I would hit f9 bring my mail program for it that feels bad let me avoid that work it's usually not by doing something better usually by reading flash down or something looking at an empty email windows is there nice boost another outworking of this system is that I've taken my poor request and I batch those kind of like ellika batched inbox I used to have this continual pitter-patter of guilty fpr's hit my box and oh I should I should be working on parsimonious and they found a bug and I'm never going to get to that make stack up so instead now what happens is I filter all those open source requests for all my projects into a an open source requests mail and I look at it once a week I look at it every Wednesday ever repeating to do says hey look at that and what do I do when I look at it I push it through that whole diagram again and I triage and you know 80% of those males get answered because they take less than two minutes and people appreciate that and my projects have picked up even though I'm not really doing any more work champion committers help I do have one of those on one of the projects whatever good things are there so so there yeah there are still things that are not two minutes we have things that I dread so much that it takes me a month or two to get to them and that hasn't changed it was all going to take me a month to get them it takes me a month now but now I don't feel guilty about it and the guilt doesn't do anybody any good so why not just accept it the other time we can feel short on time is when dealing with newcomers and newcomers are great we must embrace newcomers unless we plan to live forever and work on the same projects forever and ever and who wants to do that right yeah so but it's easy to feel like we're getting a bad return on investment bringing them up so I don't spend much time on new contribs the moment they show up I think that's all that's okay instead my investment kind of parallels theirs I think that's I think that's morally good when they first show up they're at level one they don't know where to start maybe they don't know the language the Python or HTML or whatever it is and so what do I do I say well here are some Docs you could even learn that your tutorial when in a book whatever it is I like here's the list of good first love you might want to try after that maybe I think this one is particularly appropriate to your interests and have fun and off they go sometimes they don't come back and that's fine but the self driven one's self select and they come roaring back with a patch and it's nasty and so what do I do I fix the patch I fix it for them I put commits on top of theirs I leave there's there so they get their face and github and get to feel good but I fix it up I rigged some test then I murdered and I say thank you so much please come again second time they come back they're 11 - this time I don't fix their stuff I say could you fix that and I see this and what do you think of this over here and they fix it up and they learn a little more version control and they learn the code review skills and maybe I'll write their tests for them and then we merge it in and on the third time I require them direct the tests to and then there are full fledged developer where I was getting better but at least they're doing all the things now trust Trust is such an important component of the mutual respect that we need in order to do good code review trust bank is an idea that comes out of horse training right the more you defeat the course some sugar cubes and then you have to hit the horse with the horse anything later the horse won't take it out and you would be able to take it Eno's going to be mean well it will give you the benefit of the doubt so the trouble is it's easiest to build trust between humans face-to-face for whatever legacy reasons so if why that's funny but it always is so if your remote your remote worker like I am when you're in person with your team if you're lucky enough to be make a point of establishing good relations with them you know throw the frisbee around have a meal tell good hundred jokes and then when you have to critique each other later or talk on a low bandwidth medium like IRC you'll know how to take each other you'll built up some defense against a fence if you will and then finally have these other feelings you don't know what to do with like when I look at a patch I don't like it it feels too icky to merge but I can't really articulate why at least at least I can just fall back to articulating my emotions I'm worried about this especially right here I can't really put my finger on it but something about this destination program the global right lock and feel like it's messy what do you think you don't have to be definitive decisive or authoritative all the time you just treat everybody like adults and invite them into the conversation and you dialogue and and hopefully you come to a better solution than you would have yourself just kind of just sitting there ignoring it remember you are never alone in a code review take advantage of that fact so next time you are in a code review keep these principles handy and let them help you to build your projects and build yourself and build up each other thank you so much we have time for a few questions if you come up to the microphones if you have any so if the people were reviewing feel insecure how do we make our comments sound sincere as opposed to this is something I learned in a talk and so I'm doing it right now which makes sense I mean always be sincere never never one of the word pedantic is not it never talked down to people I mean always be honest but always be kind as well so that's that's the knife-edge you want to walk these are mechanistic things for giving you ideas if you're just like I'm busy I can't think of a way to do this and the alternatives to be mean the question mark is my favorite I like rephrasing is a question that doesn't make it any less sincere but it softens it any others yeah how do you deal with knickers on your team it is nitpickers ah well I don't have the problem of nitpickers right now because I'm kind of a lone gun lately and people in my projects don't seem to be nit pickers but look it immediately comes to mind as I can say is this something we need to block the patch on or can we push it ahead into a different query you know pilot is a bug and come back to it later or we can say is there way we can automate this so that you don't have to pick that knit next time but really I think that the if those don't work and the really the base thing is to establish a good relationship with the knit picker take the knit picker to lunch not just one on one because it's kind of weird but at least go with three people and build a friendship so that then you can say really really and they give you the benefit of the doubt yeah generally if you're going to have to if you have to argue with somebody argue out loud because as soon as you write it down it gets much much louder hi thank you for your talk I really like the sort of managing your workflow and I was curious what tools you use to do that yeah I use the things app on my iPhone and Mac I use it because it's not cross-platform other good stuff is the todoist I think you have to rent that one there's two do number two do I think those are some of those are web-based those are both good though you can totally do it on paper all that's in that whole flow chart that just lists its just lists have you ever used Trello I have not used Trello yeah I'm thinking of doing like a getting things done open space later on so if come up afterwards find me help getting it out of the way and if enough people come up I'll actually do it thank you very much for the talk a lot of this stuff seems to apply in teams but you know if your team of one how would you adapt this to you know do you review yourself oh yeah do I personally know yeah tell me tell me how you how does code review apply to your lack of a team I mean I work in a team right but like I can imagine if you're a solo developer working on your own project if you have a hobby thing that you're doing in your in your off time right how do you apply this like to yourself well I mean what happens to me so I was a solo on all of my open-source projects that I started by myself and then people came in later and did a better job than I did what you don't get if you get if you don't get contributions and I do you do get support requests and you can treat them with this many of these same tools same kind of you know time-saving workflows the same kind of interpersonal stuff because they're people too even users are people I know it's a contentious issue but I'm staying by that that's all we have time for thank the speaker again please thank you [Applause]

Channel: PyCon 2017

Views: 9,759

Rating: 4.9281435 out of 5

Keywords:

Id: iNG1a--SIlk

Length: 46min 5sec (2765 seconds)

Published: Sat May 20 2017