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 an 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 wold start 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 stage of objectively going through the code, line by line and providing critique from any angle: style, refactoring, testing , decoupling etc.
All reviewers will add a comment for each of the issues he notices during the review. All comments given by the reviewers have to be resolved by the submitted 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 close 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 software engineer that is trained to look for security breaches. Should be somebody that know very well the technology stack and the architecture. The role also assumes paying attention to the current threat model that is crated against the architecture and signals those threats to be mitigated in the code.
This is the final stage which has the final checks that need to be done before pushing the code on the main line. The software engineer would run all unit tests and check for any failing tests. Would pull the latest version of the code base and merge it to his current version to get all the updates and check if there are any merge conflicts and address them. And finally, he would trigger a integration build on his current code to make sure everything works as expected. If every check is passed, then the code would just be merged to the main line.
Benefits 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 drivers 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 the 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.