Reviewing Code on Github

When you are responsible for reviewing code that someone has submitted in a Pull Request (commonly just called a “PR”) on github, your job isn’t as simple as responding “looks good!”. The goal of the Pull Request process is to ensure all merged code reflects the common wisdom of the team, so its your job to actually read the code you’re reviewing and to ask questions and provide detailed comments. This may be the last time anyone looks at this code before the rest of your team builds your analysis on its output, so take your reviewer role seriously!

Seem like there’s nothing to do in your review / everything looks great? Even when people write perfect code (which is rare!), there’s almost always some room for improvement in terms of providing better comments, using variable names that are easy to understand, or just being explicit about what assumptions one is making. In a real project, of course, you don’t want to be pedantic, but for our class exercises, be sure to find something you can engage with constructively for practice.

It’s also important to emphasize that a good PR review is an interative process, with the reviewer asking questions and the code writer responding and, usually, updating their code.

To illustrate, here’s an exceptional PR review from last year by Vanessa, our 2020 TA, along with some great clarifying responses from Fang Feng. I would like to have seen more of Vanessa’s suggestions integrated into the code before committing (some were in commits below the discussion, but not all), but this is a great starting example.

Note that much of the exchange took place through individual comments added to specific lines of code (to do this, click on the “Files changed” tab, then click on the + icon that comes up when you mouse over the line numbers next to the code you want to comment on). This only works if you submit your python code in a plain text file (.py). Jupyter notebooks will make a mess of this kind of review!