Remember, Ted? The bad programmer? We’re going to go into detail of how to be the exact opposite of Ted.
Review Attitude
Relax
In most cases, code reviews will go just fine. You’re going to get a lot of comments. Take them in stride, fix them and be open to suggestion.
Have a healthy debate. Don’t be afraid to ask what they mean by something. Don’t be afraid to ask for an example. Sometimes your reviewer will be tired and they’ll misread your code. Don’t go turning all of the for loops into recursion because Jerry needs a nap.
As long as you’re open to suggestions, code reviews will be a very positive experience.
Don’t overthink comments
Just like we said with reviewing code, you must assume good faith.
I use to work as a server in a restaurant for about four years. One of the best realizations I had about the job is that I don’t know where my guests (customers) are coming from. They might have just gotten evicted from their apartment. They might have just gotten out of a long interview that they tanked and haven’t eaten all day. They might have had a bad experience with the hosts up front. Their kids might have been driving them insane the entire day.
When a guest got snippy, angry, complained, impatient or just plain rude, it’s important to remember you might just be catching the end of their day. This might have been the final straw of many more that you didn’t see.
The same is true when they were inappropriate, pushy or challenging. They might think what they’re doing is fine. They might have just been trying to make a joke. They might have accidentally crossed the line.
The point is, there’s a lot of reasons for what people say and do, and it is always good to assume the best possible scenario. There’s very few evil people in the world, and it’s unlikely that they’re on your team. There’s far fewer people who hate you then you think.
Thus, when someone says:
This is one of the worst functions I’ve ever seen in my entire life.
Take it as a joke, because it probably is. Even if there’s reason to think they might be frustrated with you, try to joke back.
Text sucks. It’s really hard to give it the proper inflection you’re aiming for when you actually talk to someone. Sarcasm doesn’t come across right. If you’re worried about the intent of a comment, talk to them directly, assuming good faith while you do so.
Respect your reviewer
When making a pull request, you should be putting it up expecting it to go straight to master because you worked so hard to make it perfect. Of course, this will rarely happen, but the point is that it should be your goal. If you put up pull requests with the expectation that something will be “cause in review”, you’re wasting the time of your reviewer.
Every time you get comments, the reviewer has to rereview the pull request. That takes up time. They might not be able to do a second review in the same day. Don’t be shocked if your lazily put together pull request gets shot down and you have to wait a day to submit it.
Making a Pull Request
Prep
Before you even start your code, think of how you’re going to submit it. If you tunnel vision and write 2,000 lines of code and expect it to be reviewed thoroughly, expect to make another pull request fixing the bugs it’ll cause. There’s a pretty heavy fatigue that kicks in after about line 500 in a pull request. It’s not really feasible to learn how that much code actually works and catch bugs.
Do you have any external libraries? Push them first. Pushing your two file code along side all of GLFW is a recipe for disaster.
Push it in steps
Is there a basic version of the code you’re intending that’s testable? Make that and push it so it can be reviewed while you continue.
After you make that pull request, you can make a new branch off master and use the command git pull origin BRANCH_NAME to pull those changes onto your new branch. This allows you to continue working while your team reviews your code. There’s really no excuse for stopping. Just keep on making new branches and move along. That said, it’s good to give them a poke when you’re a few pull requests ahead of them.
Make detailed comments
Take the time to preempt any questions someone might have about your functions. Do you have any special expectations for the function? Why does this function exist? Who should be calling it?
What’s the variable for? Why did you use one data type over another? Are there any limitations? Can it be null?
You don’t have to write a novel for everything, but if there’s anything out of the ordinary or anything that isn’t part of the natural train of thought of a new reader of the code, comment it!
Self Documenting Code
While comments are great for context and exceptions, your code should be clear from the way you write it. The more complex the code or math you’re doing, the more time you should take to try to make it easier to read. If someone who doesn’t program can vaguely figure out what your code is doing, you’re on the right track. While this is more of a guideline than a hard rule, it’s good to shoot for it.
Make those variable names more detailed. Break functions up into better named helper functions.
Try to make your code beautiful. C++ and beautiful don’t often go together, but for your code, it should.
Give the pull request context
Even if you finished your code a day after mentioning it to someone, they might have completely forgotten about it. In your pull request, write detailed changelists so reviewers have an expectation for what they will be reviewing. This is also a great way to double check that what you say is in there is what you expect to be there! Literally go down the files changed section and add to a list as you go down it.
This is also where the labels come in. Add whatever make sense and even create some new ones if it’ll help the reviewer out.
At this point, you’re ready to review and be reviewed. Time for another YWSSS.