Sophomores are between a rock and a hard place — they are neither brand new, wide-eyed freshmen that people want to help out or the superstar seniors who impress the rest of the class with their amazing skills.
Scott D’Angelo and Andrea Rosa feel the pain of what they call the “middle class” of the OpenStack community. The pair, who both work for Hewlett-Packard, moved from being simple bug fixers and who contributed occasional reviews to getting more involved in complex features and grokking what it takes to drive things in OpenStack.
‘Our employers asked us, ‘Hey, we want this feature’ and things got more complex. You have to start writing blueprints and specs. You have to know how things work well enough to even know if this feature’s going to get in. You have to start using some influence with people to get them to even look at your patch,” D’Angelo, a senior software engineer who has worked on the public cloud since the OpenStack Folsom release, says. “We felt like there’s a bit of a gap between welcoming the new person and all the things you’ll find in the documentation about submitting your patch.”
D’Angelo is based in Colorado and Rosa lives in the U.K. but the two bridged the miles working together on a patch. Based on their experiences co-authoring that patch, aimed at getting a new feature into Nova – eventually abandoned after 53 patch sets – they gained many valuable insights on how the process works. Then they took things to another level by sounding out project team leads (PTLs), core reviewers and peers about what they look for and what annoys them.
The result? This handy curriculum for OpenStack 201 aimed at getting you out of the sophomore slump.
Know where you’re going
The people Rosa and D’Angelo sounded out said that one major annoyance was people who don’t read the history of the patch. The ire of one core reviewer was raised after he flagged something “Don’t do this. You’ve got to remove this parameter.” Then several patch sets later, the parameter came back and someone approved it and the code got merged in.
“Read the history of the patch because then you’ll know why things were addressed the way they are and you’ll avoid that kind of mistake. It can be tedious but it’s one of the things that came back in our survey several times,” D’Angelo says.
Find out what work is needed by reaching out to the PTL on IRC, he says, adding that “you’ll be encouraged and welcomed and helped more if you find some point of pain in the project that’s neglected.”
Learn how to drive in traffic
“OpenStack is a big community. It’s a lot like a traffic jam. People are helpful, people are encouraging — but people are busy,” D’Angelo says. “To get things done, you have to be as efficient as possible because everybody has a completely full schedule. There are certain things that you can do to speed things up and there are certain things that happen that slow things down.”
Swerve bad reviews with commit messages and unit tests
“Think proactively about all the problems that you’re going to cause and avoid the things that you know are going to give -1s or cause issues,” D’Angelo says, admitting that he has sometimes been his own worst roadblock by hastily pushing code without spending enough time on the unit testing or commit message. “By not being thorough, it pushes the whole process back and you get a bunch of -1s,” he adds.
One answer cropped up over and over again, the pair said. Bad commit messages — ones that don’t do a good job of explaining what a patch does — push you to the sidelines quickly. Do not expect the reviewer to open the bug description or the specs and read through the entire description, Rosa says. “All the relevant information a reviewer needs to get the context of the patch must be in the commit message. The fix to keep from annoying people with this quite simple — use the guidelines.”
D’Angelo admits that “[commit messages were] the last thing I write when submitting a patch. There’s a little disconnect there because I don’t put the effort into the commit message that I’m putting into the code — but I’ve learned to stop that practice.”
Minding the rules of the road with commit messages will save you a lot of strife, they say. Keep them to 72 characters or risk an automatic -1. “Rather than argue about it, at least know the rules and think ahead when you’re submitting your patch because someone’s going to see it immediately and tag it,” D’Angelo adds.
Pass the unit test with flying colors
D’Angelo says, “No unit tests and you get the -1 immediately. Once again, it’s one of those things where you should plan ahead — write the test first because you’re going to write the test sooner or later.” He adds that your test or patch will fail Jenkins if it doesn’t pass Tox and Pep8 first.
“I can’t count how many patches I see up there for review and then it fails and then you look at why it failed and it was hanging indent problem or limestone differentiate,” D’Angelo says.
Avoid rush hour
“One thing that was labeled as annoying is people bullying or nagging to get things reviewed,” D’Angelo says. “We see a lot of this in Cinder, because we’ve got about 70 drivers for storage arrays in the tree and there’s always new people coming on board because their company wants to get a driver into Cinder.” These drivers can weigh in at 2,000 or 3,000 lines of code and there are only so many people who have time to review.
The secret? Don’t wait until the second milestone to start nagging people about your patches and don’t wait to start asking questions in IRC and don’t wait to start getting involved in the project, he says.
The same goes for understanding the release cycles, Rosa, who considers himself a “middle-class citizen” from the Diablo release, says.
“You make some promise to your boss but remember that the timing of the community could be very different from the timing of your company,” he says. “You need to set expectations. Otherwise, you can end up like these guys — you are the one in front of the boss saying, ‘Sorry. We need to wait until the next cycle so maybe we will have everything in three months time.’ Be aware of this.”
How to fight your “traffic tickets”
If you get those dreaded down votes, don’t stall. “Be prepared to stand up for your decisions,” Rosa says. “When you submit a patch, remember: a -1 is not the end of the world. They are not always right — this goes for core reviewers but for reviewers in general.”
Clearly explain what your patch does, try to show that it is an improvement for the project or fixes a very nasty bug and you can turn the situation around, he adds. Keep an open mind, be respectful, but address specific concerns. “I’ve seen a core reviewer change their mind from a -1 to +2.”
Want to shift gears? Review, review, review
“You want to be reviewed. That’s how you’re going to get your stuff in but then again, you need to review not only to show you’re participating but that’s how you learn,” says D’Angelo.
The pair also advise using the power of zero in reviews and avoiding trying to artificially rev up your Stackalytics numbers with a lot of -1s. Getting your name known to the community — including being active on IRC — before you need something is also key.
“Core reviewers do lots of reviews. They see your reviews and they know whether you’re giving a good review and you’re really thorough in giving -1s because you find flaws in the code,” D’Angelo says. “They also know if you’re the guy who’s racked up a lot of -1s because you’re the one who catch all the typos. I don’t think you’re really fooling anyone.”
Stop sweating the small stuff
D’Angelo says that the sophomore contributor has to keep in mind what matters while trying to change the culture for the better.
“You can’t really do things about other people’s reviewing and nitpicking and getting on you for a typo or something,” he says. “As reviewers, we can make sure we don’t do this. One of the things we started doing in Cinder is encouraging people not to give a -1 for a typo or something that doesn’t affect the code. Just say, ‘If you upload another one, please fix it.’”
It’s really pleasant to get to the point where people who nitpick get chastised. “It’s starting to work,” he says.
D’Angelo and Rosa teamed up to share tips (and give hugs!) at the Summit Tokyo. Catch the complete 41-minute talk on the OpenStack Foundation’s YouTube channel.