Approach for code base reviews

[jurra]

Draft for a guide to conduct codebase review

Background and considerations

As part of our support activities, we constantly have to review research code in what way or another. Reviewing a research codebase implies reviewing the documentation of the code and the quality of the code. Documentation may include not just readmes but also how the code is commented and even written.

We are at the moment discussing and framing how to conduct code reviews, or some kind of checklist to approach code bases done by others in a systematic fashion. At the moment issues related to time, preparation or how in depth it should be are not clarified.

The guide is meant for people that have been asked to do some code base peer review

We have started drafting this for the core DCC team of DMs and RSEs but we forsee that others can also benefit from this guide.

Who can conduct code reviews?

Code reviews can be conducted by anyone that has an intermediate level of programming (Knows abotu refactoring techniques, unit testing, documentation, etc.).

Proposed approach

Premises:

• A good repo starts with a good readme, a bad repo starts with a bad one
• The readme or documentation should contain clear instructions for contributors.
• A codebase with some kind of unit testing allows contributors to know how the codebase works, what kind of functions and -abstractions have been set up and how modular the code is.
• Refactoring and enhancing code requires unit tests so that we can safely change the code.
• We are assuming that the code is transitioning from a highly exploratory and modelling phase to a production and distribution ready state. (For prototyping and exploring solutions you don't necessarily need to apply unit testing, repository documentation, or continuous integration)

Assumptions about the audience

• To conduct a review there should be the author and the peer that will review the code.
• Basic knowledge of git and github.
• The peer is on top of the domain and context in which he/she wants to use the code.
• The peer aims to distribute the code in a way that can be used by others.

Activities:

• Start with repo documentation review
• Test the documentation for users (Installation and so on)
• Test the documentation for developers and contributions

Continue to code review

• Check how readable the code is and what format they are using. Normally people start with boilerplates or templates, or go for a certain codebase structure, try to understand that.
• Check if there is unit testing, or some kind of testing, this automatically engages the peer to think about how the code is structured. It can lead to very concrete conversations and reflections.
• Take the opportunity to introduce doc strings in the context of python

Code refactoring

• Take the opportunity to do some hands on sprint planning, building progressively a backlog and then selecting those features that are worth putting in the first sprint. Introduce the idea releases.
• Take this opportunity to introduce a good branching model, I would recommend git flow
• Check the tests/specs if there are written tests, if not then,
• Start co-writing unit tests or specs with the peer before refactoring. Start ideally with one single feature test, to show the full cycle of red, green, refactor (Test driven development)
• Take the opportunity to introduce Continuous Integration of tests once new features with the subsequent tests have been added to the code base using tags and the release service of github.

Pre Release or release preparation and setup

• Teach how to conduct a prerelease or release using github,
• Roadmapping
• Help document a roadmap of next features or features available in beta
• Reflect on version and releasing cycles

References:

• Code checks from UT
• eScience Center code reviews

Please let us know if you have other thoughts, references, suggestions, or feedback.

[jurra]

Some learnings and notes from the Open Science event - Research Code reviews:

Offering central code reviews as a "service or product" is risky and some things should be considered:

Code reviews in the context of science can be difficult without (domain) context, as a lot of code is in an early stage where code structure is not a priority. Therefore reviewing code in the early phase requires a lot of context considerations. In this scenario, we have several options and considerations Is the reviewer relatively experienced with the domain? Can the reviewer easily understand what the code should do? If no, what can we do: We can focus first on reproducibility (without being able to judge the correctness of the code). We can deploy an iterative process of reusing and reproducing the code. Look for code smells without and higher levels at modules and functions levels, then go deeper only if necessary. (These is all documented above)

The determinant factor is time, effort and human resources

There is a general agreement that code reviews and better code quality is desired, but is more a question of putting effort and time than agreeing on the fact that it should be done.

Different types of code reviews (purpose is very important)

For instance, one participant mentioned, "I always start by asking what the code does".

Code reviews are a mechanism to learn best practices in a community and peer to peer approach

Encouraging people to do code

Implications for our services

• Research code base reviews should be part of our regular service and also part of our methodology to approach researchers.
• Reviews have several aspects to it, so we could have different kind of reviews (reproducibility checks, code checks, architecture or design checks)
• Other ideas?