Ashwini Oruganti – Dispelling the ‘Genius Programmer’ myth through code review – PyCon 2016

Ashwini Oruganti – Dispelling the ‘Genius Programmer’ myth through code review – PyCon 2016


(session chair)
Good afternoon everyone. We’re going to go ahead
and get started. So if you could grab your seat
that would be fantastic. Hooray, welcome.
Hi, my name is Laura Watkins. I’m the director of the Startups program
at Rackspace, and I’m the session chair
for this room this afternoon. We are very, very pleased to announce our next speaker
who is going to be discussing “Dispelling the ‘Genius Programmer’ myth
through code review.” Let’s all give a very warm
round of applause for Ashwini Oruganti. (applause) Good evening, everyone. My name is Ashwini and you may know me
from some of these places. I am a software engineer
living in San Francisco and I’m here to give
the last talk of the day. I know you’re all tired so I’ll try
to be nice and make it interesting. Ever since I was a kid I had always
wanted to be good at things without trying. The first time I came across a piano,
I tried to play my favorite song on it and somehow expected it to be all in tune
without any training whatsoever. I failed. The first time I tried to ride a bike,
I wanted to have a smooth sail and not fall. I did fall. Every time I tried to do something new,
I expected excellent results on the first attempt. Along the way, many, many falls
and failures later, it became quite clear to me
that this was not how things work. So, I hoped the same
when I started programming. You could say I was lazy
and popular culture makes success stories sound that talent is all it takes, that a successful person
has an intellectual gift, which is far greater than
what most people have in a given area. And not just in programming. It’s the same in music, or mathematics, in running, or in dancing. And by that natural ability,
they’re going to far excel the rest of us almost as if by destiny. In 2009, Fitz and Ben,
two engineers from Google, deeply analyzed this
and gave an amazing talk. They’ve crafted a myth
behind the ideal programmer. Hey, here’s a genius. The genius goes off in a cave,
writes this brilliant thing, reveals it to the world
and becomes famous forever. I mean, we all hear it so often:
“X writes excellent code. “X is really smart.” So I started investigating
what those genius programmers are really like. Where do they get their ideas? How can they look at a piece of code
and correctly predict that won’t work or that won’t scale? It is true that while being skilled
and talented does help a lot, I realize that a large part of this
is because they have put in so much deliberate practice
behind this that they’ve iterated through various, various failure modes
that when they do this now, when we look at them work,
it almost feels like they’re deciding everything
instantaneously. And when you get
to that level of specifics, you realize that there is no way,
there is absolutely no reason why these things couldn’t be learned,
couldn’t be taught, or couldn’t be practiced. A great way, perhaps the best way
to become an expert programmer is to have one teach you. Unfortunately, simply walking up to them
and asking them to train you doesn’t really work. I know this because I’ve tried. You will need to be more specific
than that, it turns out. You will need to tell them
what exactly it is that you want them to be trained in. So, a good strategy here is to ask them
to review something you’ve done. Even better, ask them to review something
related to a project they care about. Often, the projects the expert programmers
care deeply about are their own Open Source libraries. So, what I’m saying is,
go contribute to their projects. Go contribute to Open Source projects. If you haven’t already
made any Open Source contributions yet, think about how you would want
the experience to be. If you have already,
think about your experiences so far. Do you feel happy, accomplished,
or rewarded if you are — if your patch is accepted and merged? Do you feel valued if someone thanks you
for your time and work? Well, I know I do. I definitely do. And most people agree,
successful contributions make us feel happy and productive. And I don’t just mean
a successful merge here. By a contribution, I mean right from the point where one
starts working on an issue, to when the maintainers
and other community members start reviewing the patch,
to finally when the said thing gets merged. So, let’s assume that we all
buy into this. We would love to see
more successful contributions. We want our patches to get accepted and we want more quality patches
for our software. So this leads us to the next part,
now that we know that this is what we want. How do we make this process
painless for contributors and efficient for maintainers? And more selfishly,
because it all started with me wanting to be good at things,
more selfishly, how do I become expert an contributor
and how do I get more people to work on improving the library I maintain
and care about? But before we address those issues,
we have a slight problem. This whole part about non-experts
seeking feedback from the expert, this part is scary. It means sometimes showing people code
that isn’t already in a great shape. And people want to be seen as clever. And clever people don’t
make mistakes, right? There’s the — let’s take an example. There’s a thing called asyncio
that a lot of us here have been really excited about recently. It looks really good now. But taking a look into its history
shows that a lot of discussions took place during its inception. Some were in person,
some were documented online, on mailing lists, groups. There were many iterations
of design reviews, and quite some rewrites. And that, and that is how we got
to the grand state of asyncio. A lot of people were involved in it. One person alone did not write it. So, collaboration is not just essential,
it’s a prerequisite. It’s a must-have;
it’s not a nice-to-have. And, no one builds things alone. I mean, not Guido, not Jessica,
not Cliff, not Kenneth. All of these people that we know
and respect, these experienced programmers
lead their projects. They anchor its collaboration
and build a community around the things that they care about. They collect people who care
and value about the same things as them and lead them. That said, all that said,
it is sometimes frustrating to contribute when the maintainers
won’t just, like, merge your patch in. They throw nitpicks at you
or suggest an improved implementation, or maybe request a rewrite. They have a good reason to do it. Once your patch is merged in,
they are the ones who will need to maintain it in future. Their goal is to make that
as painless as possible, because all of this, because all of this
is happening on volunteer time. Yesterday there was this amazing talk here
by Augie and Nathaniel, where they showed — they started
their talk by showing a slide which had, like, never-ending lines
of poorly organized code. And the slide was animated
and pretty and everything, but every maintainer in the room
winced at it and recognized the pain
of maintaining such a project. They all — like, all of them,
I could see them praying that — hope that their work
doesn’t now or ever look like that. So every time someone mentions things
like Pepe, Linters, code coverage, continuous integration tools,
or maybe asks you for more tests, they’re not trying to block your progress
by being nitpicky, they’re just trying
to maintain code quality. And there are always things
that you can do to get those basic nitpicks
out of the way. Every Open Source project
that welcomes new contributors should have a style guide
or basically a document of all of the guidelines
that they want to see, that they want the users to follow. And every large Open Source project, like, as far as I know,
most of them should have it. If they don’t, it’s a good idea
to ask for it. But by making sure that you
follow these guidelines, you’re making the reviewer’s job
a lot easier. Also, the process that is documented
may be wrong, or it’s likely that you are
missing some context. But I would recommend saving
the camelCase fight for later. (laughter) Never forget that the reason
you’re doing this in the first place is to learn. So once you know that you have
learned all of the context that there is behind this,
that’s the right time to pick fights or argue your way or discuss things,
and people will respect you enough to not discard your opinions. So, save that fight.
Respect the process when you start. I know every potential code reviewer
in this room will thank you for this. It makes large patches
make their lives harder. And this is not just something we say
to new contributors. This is something
we need to remember throughout. For instance, someone wrote
an excellent improvement to the logging system in Twisted. The only problem was that diff
was extremely large. And so, despite having
an active development community, that diff sat
in the review queue for ages. It did get merged eventually
and we really, really — there was this one person who sat down
and, like, did the review in 24 hours, after, like, months and months of it
sitting there quietly getting no love. But gathering feedback early
is always nice for you, for the reviewers, and ultimately for the project itself. Even if things don’t feel complete,
show your code. It’s the right thing to do. And that said, you should —
like, gathering feedback early means that you can work
with your reviewer. “Respect the maintainer’s time”
is the basic message of this procedure, but it’s also a really good way to learn. You can ask them to elaborate
on their comments and suggestions. Say at some point you were asked
to throw away everything that you’ve done, the weeks and all that coffee
that you drank, and like, someone just asks you
to throw your code away and rewrite everything. That’s sad. Gathering feedback early means
you’re allowed to ask them to elaborate on their comments
and suggestions. You can afford to actually
make a case for your code. And who knows?
Maybe you were right all along. Maybe the expert programmer
made a mistake. Don’t you ever find bugs in software
written by expert programmers? So, criticism is not evil. It is hard to take. And it is better taken
when given as feedback. But even when it is presented
without judgement, it’s still just hard. So, knowing — always remembering
that the reason why you’re actually contributing
is because you want to be — you want to be a genius,
that helps. And I’m not just saying this. When I started working on Twisted,
I would get — I promise this is only slide
with the ugly screenshots. So when I started out,
this was one of my first reviews, and the same page has so many
whitespace comments. And because I was getting
the same feedback over and over again, I realized that I was making
a systematic mistake. I was still getting used
to my text editor. And I figured that maybe there must be
some way to automate this. So I found a systematic solution. This review shaped the way
for my future system setups as well. Now every time I set up a new system,
the first thing I do is to go configure my editor and automate
as much code cleanup as possible. And I rarely see review comments
like this. I rarely saw them again on Twisted,
or on any other project. Embrace failures. It’s easier said than done, I know. I am still pleasantly surprised
that everyone in the Twisted community associates me with all of the
cool things that I built and not with my whitespace mistakes, because that’s how I remember
my first week in Twisted as. But what’s important is
you learn from the mistakes and don’t fail
in the same thing repeatedly. Rather than being embarrassed by them,
consider them to be context around your thought process. When you look back, surely, surely
there will be cases where you’d go, “Hey, that was some piece
of complete nonsense that I wrote. “What was wrong with me?” But then there might also be cases of,
“Well, that approach led nowhere,” or “It makes the system fail.
Let’s not repeat it.” Or better still, you might say,
“Ooh, that’s why I did this.” The context might help you
in fixing broken parts later. So, that was all a lot of guidelines
for new contributors. I also am here to talk
to my maintainer friends in the room. So hello there. (laughter) It is really hard to see someone submit, say, a bug fix
which is completely wrong. You know you’ve seen this a lot. You know, you’ve been doing this for years
and you know how this ends. But it’s still — it’s not easy to ask someone to completely reject
their premise. And make sure the — making sure
that the communication is clear and effective
is really important. It should also be non-personal
and educational. But when I say “non-personal,” I say that the only personal part
of the review should be the part where you thank them
for their time and effort, where you value their contribution,
and I will come to that soon. But, yeah, like when my whitespace
changes were highlighted, no one assumed that I didn’t know
anything about computers just because I don’t know
how to fix whitespace in my code. And that was valuable.
It was presented without judgement. Also, if you care deeply about a style,
you should document it visibly. And I’m not just saying this
as a best practice. Twisted has about, like, five documents that are presented as introduction
for new contributors. We have spent, like, years
trying to merge them all into one. I think I wrote the sixth one,
which was supposed to be the combination of all the five.
It never got finished. So we now have six.
I stand corrected. So, make it visible.
Don’t confuse the contributor. And now whenever someone
comes up on IRC and asks for a contributing
guideline document, we — it just depends on — it’s completely arbitrary,
which i still get. People might miss things. People might overlook
seemingly obvious points. That does not make them blind. That does not make them be deserving
of your wrath. So don’t be unkind about it. While working on this talk,
I went around talking to my maintainer friends
and I asked them what was their
worst code review experience like? And my friend Ian, who maintains
a lot of Open Source libraries, including requests, quoted this review
from requests that he wrote. The first line was this. And it did not end well. I’m not showing this example
as, like, the ideal review. This is the review that he said he regrets
the most, and I could see why. This led to the person on the other end
asking for more context around his criticism, why he thought things were wrong. And he explained it,
and, I mean, it ended — the discussion ended with them agreeing
that, yeah, that’s fine, I will not do it. But it would all have been
so much more pleasant had he just started out with the — with what he wanted to say
in a nice way. So, contrast this with another review
that I got when I was working on Twisted. There was this —
there was some logical error that I was making within, I think,
with how I was organizing my if/else statements,
and my reviewer mentioned the change that he would have liked me to make,
provided a self-contained example of what I was doing
and what I was being — what was being suggested that I do. And then ended with a description
of what the change would imply, why it would be more performant,
or why it was a better idea. And I agreed. I saw the point that he was trying to make
and I never made that mistake again. So treating them
as reasonable human beings you can have
logical discussions with is really important. Make your case,
describe the change that you’re suggesting,
explain your thought process; it helps. And now that you know
that they’re there to learn, to be you, it should be more —
easier to have — to understand their motivations
of contributing to this thing that you care about. So do that. This was another — this was another example
of my code review — one of my code reviews
where my reviewer — this had been going on for weeks. I was working on this really hard thing
and it started with a “thank you.” And I was like, “Okay.” And then it started with —
and then it went on to say that this looks really good,
and I was happy. And then it mentioned how it requires
a lot more changes, which is fine
because I’m already gloating now. And then it says
that I am in the right direction. Okay, that’s a reassurance. So, this is like, such a good way
to start a review. You’re saying that –
you’re thanking them, you’re valuing their time,
and you’re working with them. You’re encouraging your contributors. And you’re also telling them early
if they’re taking the right or wrong approach so they know when
to abandon a particular track. Just make sure that they don’t just quit
or, like, do a table flip or just vanish from the Internet
for some months or weeks and the code request just sits there. So, all this said,
it’s really important that both these sides of the coin
work together. I was also at some point working
on a DLS implementation in Python. And I was pretty much the only person
working on it, so I can almost say that I was expected to be
a maintainer for it. And the change was —
the change taught me a lot of things, some of them being
that there’s no such thing as not being qualified enough. For all of the years
that I worked on Twisted, I did very few code reviews. At least, in the first two years,
I think I hardly did one or two. It was not because I was —
it was not because I didn’t have time or I found it annoying to review code,
it was because I was there to be an expert programmer. And my assumption was
that anyone reviewing my code is an expert programmer. So how do I review code then? I got such thorough reviews. They picked every potential mistake,
or anything that I overlooked, and I was never sure
that I could do the same to their code. And it took a lot of encouragement
for me to realize that it’s all right to miss things. They — again, what’s happening here
is deliberate practice, and that’s why
there’s a community to work with. So, once again, contributors,
you will need to work with the maintainers and meet them halfway,
and my maintainer friends, you will have to meet
the contributors halfway. Be nice, kind, and respectful. Thank you. [applause] Do we have time for questions? (Laura Watkins)
If there are no questions, we’ll go ahead and conclude.
Let’s give her one more round of applause. [applause] (Ashwini Oruganti)
Thanks, everyone. [applause]

1 thought on “Ashwini Oruganti – Dispelling the ‘Genius Programmer’ myth through code review – PyCon 2016”

Leave a Reply

Your email address will not be published. Required fields are marked *