Throughout the years I have been part of multiple teams and software engineering cultures. When I started out, projects were run in a pure waterfall model. Work was done through a ticketing system and estimations would be something that the manager asked you: “When do you think this can be done? How many days? Can we do it faster than that?”.
Since then, the Agile Manifesto proposed a new way of handling projects and has grown and gained popularity to the degree that is now the “de facto” standard for project management in software development. Sure, there are a lot of flavors of Agile. Each team might work in a way that only suits their needs not everybody else’s.
But the idea that I resonate the most with from the Agile methodology is the empowering of the software engineering team so that they can go fast. Delivering software has become faster than ever. Release cycles have changed from a cadence of several years (Windows OS) to yearly to monthly, weekly daily and even multiple times per day!
Going fast also implies having quality – you can’t write software fast if you go through regression or have stabilization phases.
Quality – from the engineering point of view means a healthy continuous integration pipeline that would run all types of automated tests but also a strong code review process that would keep the feedback loop as close as possible to the code development phase.
Below is a UML diagram where I have tried to define a process for code reviews that would yield the best quality at the end of a coding task (an agile story).
There are some assumptions for the tools involved: source control is done using Git and the code review tool is Crucible from Atlassian but these can be replaced with any other set of tools.
I have defined 4 swim lanes that represent different stages that a story goes through to be considered code complete and ready to be pushed to the main repository: development, code review, security review, finalization.
This is the stage where the software engineer writes code. It is the stage that will be reached if any of the future check conditions required for completion will fail. The fail conditions are represented by the red arrows.
Work starts by preparing a fresh branch and making as many commits as necessary to complete the story by respecting all the acceptance criteria.
When everything is completed, a code review is created in Crucible and assigned to the 3 reviewers. As a 4th reviewer, we will also add the email distribution list for the whole team. This is for visibility. Over communication is better than no communication.
This is the stage of objectively going through the code, line by line and providing critique from any angle: style, refactoring, testing, decoupling etc.
Reviewers will add a comment for each issue they notice during the review.
All comments given by the reviewers have to be resolved by the submitter before continuing.
The submitter will add more commits to resolve the comments and re-update the Crucible review. Once all the 3 reviewers give sign off, then the security reviewer can be added to the review.
This stage is something that you might not find today in most of the development teams but I consider it a separate mandatory step. Usually, this kind of security assessment is done before a release but I believe in the closed feedback loop and if things can be evaluated early than we will not need to rework later down the road.
The security reviewer role is usually taken by a senior software engineer that is trained to specifically look for security breaches. Should be somebody that knows very well the technology stack and the architecture and can have the hacker mindset.
The role also assumes paying attention to the current threat model that is created against the architecture and signals those threats to be mitigated in the code.
This is a stage where the final checks are made before pushing the code on the main development line.
The software engineer must run all the gated checks and address any found issue including test failures, coverage decrease and merge conflicts. And finally, he would trigger an integration build for his code candidate to make sure everything works as expected. If all of the steps are passed, then the code would just be merged to the mainline.
Benefits were drawn from this process
The questions that we would be able to answer by leveraging this process are :
- how confident are you on the code that you have submitted?
- can the code be immediately promoted to production?
The result of this whole process is confidence for the software engineer behind the driver’s seat. It also means that other people have knowledge of the code that has been submitted and this knowledge is distributed. For the more junior engineers, this process would become their learning process because feedback would be fast and on point. For the more senior engineers, this is where they would practice mentoring skills. Decisions made during reviews are documented and are saved for later reference without adding comments in the code base that could quickly become obsolete.