Source Code Control Best Practices at Endigit
GitLab sent out an email today (as I’m writing) about best practices for source code control for collaboration.
GitLab’s five best practices are:
- Determine a branching strategy
- Make frequent small changes
- Write descriptive commit messages
- Develop using Branches
- Conduct regular code reviews
As I looked over them, I was impressed at how well their best practices aligned with the best practices Endigit had developed internally from our experience. I want to brag on our team a bit, so here are some guides we’ve created and how they line up with GitLab’s best practices.
Point 1 - Determine a branching strategy
At Endigit, we adopted the Git Flow branch strategy, with a few adaptations to working largely in LabVIEW. We keep features small - usually on the level of a task an individual engineer takes on. We also have to communicate a lot because Git sees LabVIEW as binary blobs - so that we don’t end up causing conflicts with each other’s changes. But with near-constant integration on the develop branch, this method works well for us.
Points 2 and 3 - Make frequent small changes and Write descriptive commit messages
To show how well we do this, check out our internal guide around commits:
Points 4 and 5 - Develop using Branches and Conduct regular code reviews
We put a second pair of eyes on just about every coding task with code reviews. We typically have a feature branch where developer A works on a task. When they are ready to merge it in to develop, they create a merge request and assign it to a reviewer, developer B.
Developer B follows our Software Peer Review Procedure. Here’s some snippets from it. First the rationale:
We believe all code should be peer reviewed. This is to ensure quality code for our customers and to catch problems with the code as early as possible. The intention of these reviews is to ensure our code is well documented and follows our LabVIEW Development Guide.
In addition, we view peer reviews as an opportunity to learn and improve by getting another set of eyes on your code and hearing another’s suggestions. All developers should engage in peer reviews both as the reviewee when they are requesting to merge their code into the working branch and as a reviewer for someone else’s code.
And second, the questions that are looked at in the review:
- Is the code correctly documented? (VI documentation, block diagram documentation, external documentation, etc. as appropriate)
- Does the code follow the LabVIEW Development Guide?
- Is this the right change? Is this a band aid? Does this address the real issue?
- Does the code solve the problem it was created to solve?
- Does the code change break existing code?
- Does the change mesh with the original project architecture and style (i.e. non actor framework code in a non actor framework project)?
- What is the appropriate level of testing and has it been completed?
- Is there a simpler way to implement this?
- Is there any code smell (does our experience indicate to us that there’s a potential problem with the solution?)
- Is there teaching feedback that the reviewer can provide (i.e. rather than polling the UI controls, use an event structure)?
- Is there scope creep?
So there you go - when a market leader in source code control comes out with best practices, it’s a good idea to follow them. When you find your team is already well on top of them, celebrate! Then keep up the good work. And with that I’ll quit blogging and go back to work on my other project.