When and how to do code review

[noamross]

I think it would make sense to include a section on code review. This applies to "managing contributions of other developers" and the other components of feedback.

Some desired outcomes might be:

• Identify an appropriate-sized unit of code for review
• Prepare a contribution for review
• Know different modes of review (small feature/release/full project, in-person/text)
• Provide constructive feedback in a code review
• Mechanics of reviewing via a Pull Request system

Perhaps this gets outside the scope of the lesson, but one area to consider is how this differs across the spectrum from development of a large software project, where a unit of review might be a feature, and a large research project, where a unit of review might be an analysis.

[gvwilson]

I'd really like to, but I don't know how to teach it except by example - showing people checklists of things to look for doesn't seem to have much impact. What have you seen/done that's been effective? Cheers, Greg

[noamross]

We don't explicitly teach, but anecdotally, new rOpenSci reviewers do best when given a checklist and an example review that shows checks and comments.

@ctb has a blog post about a 2-hour lecture + activity on live review he gave undergrads: http://ivory.idyll.org/blog/2016-teaching-code-review.html

It looks like @abostroem has taught the mechanics part in some workshops. Perhaps she has some thoughts? https://github.com/abostroem/2015-01-03-aas/blob/8d601635fbbf59b6e70687ba8d51e056a6ffcc4e/novice/extras/03-review.md

I'll think some more on this.

[abostroem]

I've included code review in 3 workshops but @jiffyclub, @lonnen, and @cavestruz actually taught the lessons (here are the lessons not linked to above: https://github.com/abostroem/2014-08-14-stanford/tree/gh-pages/stanford/code%20review, https://github.com/abostroem/2016-01-03-aas/tree/gh-pages/code_review). Perhaps they have some comments.

For me the biggest reason I teach code is to start getting people showing and talking about their code. I think one of the most important things that students can do when they go home is build a community at their home institution that meets regularly to talk about their code (not just their research). I think many beginners think they don't know enough to contribute to a code review or think that their code is too bad to show to people. My hope in doing a code review is to give students a model to take back to their home institution about what a discussion of code can look like and to demonstrate to them that they have a lot to offer a group and can learn a lot from sharing their work.

[lonnen]

It's been more than a year, but this is what I recall of how I taught it with @abostroem --

Before class, take the exercises from Day 1 and modify add a single new concept. Rewrite portions with an altered style, introduce some incorrect behavior, and at least one outright error.

During class, establish code review as the most effective tool for increasing code quality, and for growing skills. Set expectations for the exercise. Provide the students with printed handouts of this code and give them a scenario: you return to your lab and share this with your colleagues, and one of them comes to you about a week later and says, "I have playing with the code you showed us, and was wondering if you could look it over for me."

Give them a small time box to examine the code and provide notes on the paper independently.
Give them a larger time box to compare with neighbor(s).
Give them the same larger time box to compare with totally different neighbor(s).

Bring it together with a small group recap. Reiterate that regular code review is analogous to peer review, ideally with short cycles. Advocate it as a good way to progress the skill of the group, and as an effective means of creating value and community. Introduce the idea of code review as integrated into github, if appropriate.

I like this because it gives neutral code to work on. It reinforces the previous day's material in a novel way, and encourages semi-random mixing. One of my goals was to make people feel like they were leveling up even during the exercise.

I strongly believe these lessons of community, peer teaching, and peer review are more important than any skills we can convey in a few days. It sets an important pattern.

[gvwilson]

_episodes/15-review.md has some notes on code review - I'd be grateful for feedback.

[noamross]

I like the emphasis on the point that review has the benefit of sharing knowledge - both general knowledge and understanding of how parts of the project works - especially for bringing new people up to speed. It's a surprisingly good way to foster interactions.

One thing perhaps to add is that both implementation and usability are in-scope for code review - feedback from people less familiar with the code or end users can be particularly valuable.

[gvwilson]

Partly addressed in 9d8bb31

[noamross]

Great. Perhaps this is what you mean in the second bullet, but I should have explicitly said that review includes documentation.

[ctb]

I'm not sure I agree with the first sentence - maybe citation needed? :) But it's not obviously wrong, either...

Code coverage analysis of automated tests could be included somewhere as a particular target for code review.

Personally, I would guess writing a few basic tests, and then targeting a few more by looking for missing code coverage, is probably the best (most cost effective) way to find bugs. But I don't have any evidence.