Dispelling The 'Genius Programmer' Myth Through Code Review
This is a modal window.
The media could not be loaded, either because the server or network failed or because the format is not supported.
Formal Metadata
| Title |
| |
| Title of Series | ||
| Part Number | 15 | |
| Number of Parts | 52 | |
| Author | ||
| License | CC Attribution - ShareAlike 3.0 Unported: You are free to use, adapt and copy, distribute and transmit the work or content in adapted or unchanged form for any legal and non-commercial purpose as long as the work is attributed to the author in the manner specified by the author or licensor and the work or content is shared also in adapted form only under the conditions of this | |
| Identifiers | 10.5446/32688 (DOI) | |
| Publisher | ||
| Release Date | ||
| Language |
Content Metadata
| Subject Area | ||
| Genre | ||
| Abstract |
|
00:00
Goodness of fitMultiplication signSoftware engineeringComputer programWave packetChannel capacityResultantMachine codeAreaComputer programmingRight angleMusical ensembleContent (media)Natural numberEvent horizonMathematicsSpecial unitary groupComputer animation
03:06
Projective planeComputer programExpert systemStrategy gameAsynchronous Transfer ModeMereologyFeedbackOpen sourceMachine codeLevel (video gaming)Library (computing)Latent heatScaling (geometry)Object (grammar)Group actionComputer animation
05:00
Software maintenanceContext awarenessMachine codeStapeldateiContinuous integrationImplementationMultilaterationProcess (computing)MathematicsPoint (geometry)Projective planeRewritingMultiplication signPatch (Unix)1 (number)Text editorIterationArithmetic progressionOpen sourceCollaborationismLibrary (computing)Software testingPhysical systemThermodynamischer ProzessNatural numberExpert systemComputer programMereologyEmailSlide ruleElectronic mailing listDifferenz <Mathematik>Queue (abstract data type)Software developerVector potentialWhiteboardFeedbackLine (geometry)View (database)SpacetimeOperator (mathematics)State of matterAreaObject (grammar)SoftwareRight angleWordShape (magazine)VolumenvisualisierungINTEGRALVariable (mathematics)AverageKey (cryptography)Spring (hydrology)Extension (kinesiology)Parameter (computer programming)Bit rateFile viewerRoundness (object)Pattern languageDirected graphVideo gameComputer animation
14:33
Thermodynamischer ProzessMereologyExtension (kinesiology)Machine codeStapeldateiLine (geometry)Goodness of fitBranch (computer science)Projective planeImplementationContext awarenessCASE <Informatik>MathematicsError messageCategory of beingComputer programPerspective (visual)Vector potentialSoftware maintenanceOpen sourceExpert systemRight angleStatement (computer science)Physical systemMassTrailMultiplication signPoint (geometry)Associative propertySingle-precision floating-point formatDirection (geometry)ResultantUnit testingAnalytic continuationState of matterBuildingInferenceString (computer science)Patch (Unix)Dimensional analysisView (database)Simulated annealingLibrary (computing)Computer animation
22:56
SpacetimeRoboticsElectronic program guideMachine codePerspective (visual)Projective planePoint (geometry)Computer programmingMultiplication signThermodynamischer ProzessParticle systemBit rateAxiom of choice
24:06
Computer animationXML
Transcript: English(auto-generated)
00:17
Good morning, everyone.
00:22
I'm a software engineer living in San Francisco. You may know me from one or more of these other places. This is my first DjangoCon ever and also my first time in Philadelphia. So I'm really excited to be here. The campus is, like, amazing.
00:41
So 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 I somehow expected to get it all right without any training
01:02
whatsoever. I failed. The first time I tried to ride a bike, I wanted to get it right once again and not fall at all. I did fall. And I say these things as if the older me is any wiser,
01:21
but that's not really true. So every time, even today, when I try to do something new, I still hope to get excellent results on my first attempt. Many such falls and failures later, now I know that this is not how things work.
01:43
I had hoped the same when I started programming. It was like, you can say I was lazy or that popular culture makes success stories sound like talent is all it takes, that a successful person has an intellectual gift that is far greater than what most
02:03
people have in a given area, and not just programming that's like mathematics, music, dancing, running, any athletic sport. And by that natural ability, they're going to far excel the rest of us, and almost as if by destiny, right?
02:21
You cannot question destiny. So in 2009, Phuocen Ben, two engineers from Google, deeply analyzed this. They've crafted a myth behind the ideal programmer. You say, hey, here's a genius.
02:40
The genius goes off in a game, writes this brilliant thing, reveals it to the world, and becomes famous forever. I mean, I hear it too often. I'm sure those of you who are software engineers in a professional capacity also hear it too often.
03:02
X writes excellent code. X is really smart. So I started investigating what these genius programmers are like. Where do they get their ideas from? How can they look at a piece of code and say, hey, that won't work, or hey, that won't scale?
03:26
It is true that while being skilled and talented helps, I realize that a large part of this is because these experts have put in a lot of deliberate practice behind their work.
03:43
They've iterated through so many failure modes so much that now when they work, it all appears to be instantaneous. And when you get to that level of specifics, you realize that there is no reason why these things cannot
04:03
be taught, practiced, or learned. A great way, perhaps, the best way to become an expert programmer is to have one teach you. Unfortunately, just simply walking up to one
04:21
and being like, hey, teach me, make me like you, that won't work. That doesn't always work, at least. I know this because I've tried. So a good strategy here is to ask them to review something that you've done and gather feedback.
04:40
And it's even better if that something is a project they care about. And often, the projects that programmers care deeply about are their own open source libraries. So go contribute to their projects.
05:02
If you haven't already made any open source contributions yet, think about how you would want that experience to be. If you have already, think about your experiences so far. Do you feel happy, accomplished, or rewarded if your patch is accepted and merged?
05:23
Do you feel valued if someone thanks you for your time? I know I definitely do. And most people agree. Successful contributions make us feel happy and productive.
05:40
And I don't just mean a successful merge by a contribution. When I say contribution, I mean right from the point where one starts working on an issue to when the maintainers or other community members start reviewing their code to finally when the said contribution gets merged.
06:01
So let's assume we all buy into that, that we all want to see more successful contributions. And we want our patches to get accepted. Also, as maintainers, we want more quality patches for our software. This leads us to the next part. How do we make this process painless for contributors
06:24
and efficient for maintainers? And more selfishly, how do I become an expert contributor? And how do I get more people to work on improving the library I maintain and care about?
06:42
Before we address those issues, we have a problem. This whole part about a non-expert seeking feedback from the expert, this part is a little scary. It means that you have to sometimes show people code
07:02
that you wrote that isn't already in a great shape. And people want to be seen as clever. And clever people don't make mistakes, right? Wrong. Well, let's take an example from Python.
07:21
There is this library called, there is a part of Python called asyncio, which a lot of people have been really excited about recently. It looks quite good now. But taking a look into its history shows that a lot of discussions took place
07:40
during its inception. Some were in person, some were online, some were on mailing lists. And there were many iterations of design reviews and quite some rewrites. And that is how we got to the current state of asyncio.
08:00
A lot of people were involved in it. One person alone did not write it. Collaboration is not just essential. It's a prerequisite. It's a must have. It is not a nice to have. And no one builds things alone, like not Guido, not Jessica,
08:24
not Cliff, not Kenneth. And the experienced programmers just lead their projects. They encourage collaboration and build a community around the things they care about. That said, it is sometimes frustrating to contribute
08:44
when the maintainers won't just merge your batch in and throw nitpicks at you and suggest an improved implementation and request a rewrite. But they have good reason to do it. Once your batch is merged in, they
09:02
are the ones who will need to maintain it from there. Their goal is to make that process of maintaining it as painless as possible. Because all this is happening on volunteer time, especially for open source libraries that have been here for a while.
09:21
By God, this year, there was an amazing talk by Augie and Nathaniel, where they showed a slide with never-ending lines of poorly organized code. The slide was animated and pretty and really well-written. But every maintainer in the room winced at the sight of that slide.
09:42
They recognized the pain of maintaining such a project. They all secretly hoped that their work doesn't now and doesn't ever look like that. So every time someone mentions Pepe, linters, code coverage, continuous integration tools, or just more tests or more
10:04
documentation for tests, they are not trying to block your progress by being nitpicky. They are trying to maintain code quality. And there are always things you can do to get around those nitpicks. For example, look for coding standard guidelines.
10:23
Every large open source project welcoming contributions should have one. But making sure your code follows these basic guidelines before you submit it for review, you're making the reviewer's job a lot easier. And before you actually go on to contribute,
10:46
knowing the process is useful. At the same time, it's also important to respect the process. Many open source libraries have these well-established steps or processes that they like following.
11:02
Sometimes it can be hard to understand why this process was chosen or why it is so important. The process may be wrong. It may look wrong. Or you may be missing some context. I would recommend saving that fight for later.
11:21
Never forget that the reason you started doing this in the first place was to learn. You can always go and teach the process or modify it or ask questions. You can ask questions even initially. I don't think anyone would mind that. But as about the fight to change it,
11:42
you can save it for later. I know every potential code reviewer in this room would thank you for this. It makes their lives very, very difficult if you submit a huge board request with lots of changes.
12:02
And this is not just for new contributors. I will take an example from Twisted where someone, a longtime contributor, made some really amazing changes to the logging system of Twisted. The only problem was that that diff was extremely large.
12:23
And so despite having an active development community, the diff sat in the review queue for ages. We finally had to dedicate one whole sprint day at PyCon to review it and get it merged. So gather feedback early.
12:42
Even if things don't feel complete, show your code. That's the right thing to do. Also, respect the maintainer's time. It's OK to ask them questions and ask them to elaborate on their comments and suggestions, but respectfully, not in an argumentated manner.
13:01
And who knows, maybe you were right all along. Maybe the expert programmer made a mistake, right? It happens. It happens a lot. I say all this, but I know criticism is hard to take, even when it is presented objectively without judgment or prejudice in the nicest possible manner.
13:24
And that's really just human nature. Let me show you an example from one of my first contributions to Twisted, where I was working on implementing some endpoint APIs. And there was this one thing that kept showing up
13:41
on my code reviews repeatedly, whitespace. Because I was getting the same feedback over and over, I realized that maybe I'm making a systematic mistake. Maybe the answer to this is not me going and fixing whitespace. Maybe I need a systematic solution for this.
14:03
So I dug around. I asked the experts and figured that it is possible to configure your text editor to take care of the whitespaces. You don't have to bother. And 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
14:21
is to configure my text editor to automate as much code cleanup as possible. And I rarely saw review comments about whitespace again for that project or any other. So embrace failures. It is easier said than done. I know. I am still pleasantly surprised that everyone
14:41
in the Twisted and Python community associates me with all of the things that I've built and not with my whitespace mistakes. But what's more important is that you learn from the mistakes and don't fail on the same thing repeatedly. Rather than being embarrassed by them, consider them to be context around your thought process
15:02
and document them intensively. Then you look back. I know documenting mistakes is not easy. When you look back, surely there'll be cases where you'll go, hey, that was some complete nonsense I wrote. What was wrong with me?
15:22
There might also be cases of, well, that approach led me nowhere. Or it makes the system fail. Let's not repeat it. Or you might say, oh, that's why I did it. It makes sense. That context might help in fixing broken bytes later. So that's a lot of things for contributors.
15:42
My talk is not just for them. I also have some things to say to potential code reviewers and my maintainer friends here. So hello. When you communicate, are you clear and effective to understand? Are your reviews non-personal and educational?
16:04
Well, when I say non-personal, that is for the review on the code itself. The only personal part of the review should be the part where you thank people for their time and effort. And that part should definitely exist.
16:21
People have their quirks. I, for example, like to test docstrings to the extent where I tend to not look at the accompanying code itself until the batch has unit tests with docstrings that tells me exactly what's happening. This may or may not always be important for all projects. But it matters to me.
16:40
So when I build and develop a library, when I maintain a library, I will know that docstrings are important to me. And I will likely look for them in any pull request. That sounds fair if I just document that in the contributing guidelines because I'm the maintainer and I have clearly communicated a prerequisite
17:00
for making changes to my project. It is not enough to just document it, though. You have to also make it visible. listed, for example, had like five contributing documents that were developed over the years. And we tried for a long time to make
17:20
build one single document out of them. I think I wrote the sixth one that was aimed at collaborating all five. And that still hasn't been completed. So we have six contributing documentation. That's not really visible. And that's also not the right way to go about telling your contributors what you care about.
17:42
And still, even if you do that, even if you document it visibly, people might miss things. People might overlook some seemingly obvious points. That does not make them blind. Please don't be unkind about it. They do not deserve unkindness for this.
18:02
I was going around and asking my friends about their worst code review experiences. Ian, who maintains a bunch of open source libraries, including Requests, said that one of the reviews he regrets the most is from the Requests library.
18:20
In a pull request, there was one point where instead of describing why something was a bad idea, he just wrote this below the code. And the contributor politely asked for more context behind all the notes. And he explained, this line was fixed
18:41
and the pull request was merged eventually, but Ian said that things could have gone so much better and smoother had he objectively described the point the first time itself. It might not feel like it's important while you're doing a code review if you've seen the same thing being done
19:00
over and over again, but it's still worth the effort to explain why. Okay, this reminded me of the time when I was making a logical error in my code, which was not so obvious to me. I think it was about how I was organizing
19:21
my if-else statements and my reviewer, Mike, the change provided a self-contained example to help me understand the error that I was doing and described in detail why this was important. And I never made the same mistake again. It is important to treat people as reasonable human beings
19:43
with whom you can have a logical discussion. Make your case described by the change you're suggesting is necessary. On this slide, there's a review that I once got for a super complicated implementation
20:01
that I was doing for Trusted. I'd been working on this for weeks and when I submitted it for review again after a lot of attempts, this is what I got. It started with a thank you. That's nice. It says that things looked good. That's nicer.
20:21
It then mentions that my branch needs a lot of changes. Well, that's okay, because I'm already happy from the first two sentences now to actually care about what the next line says, really. And it even ends with a smiley. Like, who knew expert programmers use smileys?
20:42
It also reassures me that I'm going in the right direction. So I think this was a great gold review. I think that even though the changes took me days to complete and it was a while before this thing was merged, I feel like the kind of happiness this review gave me was really nice and rarely seen.
21:05
So work with your contributors. Be encouraging. Thank them for their time. And tell them early if they're taking the right or wrong approach so they know when to abandon a particular track. I have said a lot of things in the last 20 or so minutes to both contributors and maintainers.
21:22
And I know these things because I was a contributor for a long time before putting on my maintainer hat. And that new hat taught me a lot of things and brought with it a massive change in perspective. The most important one being that you're always qualified.
21:42
For many years, I did not review a lot of twisted code. I contributed a lot. I built a lot of things. I responded and participated in discussions, but I rarely reviewed actual code from other people. The reason being that the people that I was working with were excellent expert programmers.
22:01
And their reviews were thorough, educational, and just so awesome. I thought that that was a prerequisite for reviewing code. And I did not feel I was knowledgeable enough to do that. What I didn't know was that good code reviews also come with practice. Being an expert isn't a prerequisite. It is a result of continuous practice.
22:23
And that's why there's a community to work with, right? So once again, contributors, you will need to work with the maintainers and meet them halfway. And my maintainer friends here, you will have to meet the contributors halfway. Be nice, kind, and respectful.
22:43
Thank you. Thank you so much, Ashwini, that was amazing. We have time for one question. Hi, fantastic talk. I'm interested in getting from the perspective of project maintainers, and I suppose also from contributors.
23:03
What's your opinion about automated tools for doing some of these review process? Things like the white space trailing and even some of the star guide stuff that Trey was talking about in the previous talk can be picked up automated and you can write little programs that'll just jump in automatically and say, oh, by the way, you've got trailing white space. Oh, by the way, this thing is more than 80 character
23:20
or whatever the problem happens to be. That's incredibly impersonal. It's completely robotic, like it's a robot. Do you think that's a good approach or a bad approach for a project to adopt? Thank you for your question. I definitely think that's a really good approach. This whole, like all through this talk, the point that I've been trying to convey for reviewers
23:43
was that your review needs to be non-personal. No one takes what a robot says personally. So I think if the more robots in your code review process, the better it is, because the more non-personal your code review is.
24:01
All right, thanks so much.