At VSHN we’re doing a lot of engineering. To have a common understanding and set our level of expectation, we describe our way of engineering in this document.
We strive for good engineering quality. This implies getting and doing proper reviews before merging code and other produced artifacts (e.g. documentation).
When doing review, stick to these points:
Clearly indicate which change requests are blocking and which requests are nitpicks.
The PR or MR author is allowed to merge the PR or MR if it’s approved by at least one reviewer.
Before major engineering is done, we write design documents. For now, we have the following formats:
By doing that we ask the right questions beforehand and have a track record why we did something in a particular way.
This is best done with AsciiDoc and Antora, in one of the sites at kb.vshn.ch. By using that approach we can achieve a well-structured documentation and by leveraging Git a proper review can be conducted.
Proposing changes or additions to an already existing code-base should be done in a way that helps others to understand what it’s all about:
Write PRs or MRs for a single logical change. They can be large, but they shouldn’t contain major changes that are better done in a follow-up or preceding PR.
Put smaller changes like fixed typos or whitespace into their own commit.
Don’t mention issue numbers in the title or the branch name on public repositories, such as on GitHub.
Ensure that the title is meaningful. The title will (can) be used as an entry in the changelog.
Use the imperative mood for both git commits and titles.
Open PRs or MRs in the draft state if the work is still in progress and not yet ready for review.
When the PR or MR is ready for review, request a review from at least one other team member, whenever possible two of them. If you don’t want a review from a specific person, please assign the PR to a team. This will automatically request reviews from two random other team members (only available on GitHub).
Only the PR or MR author merges the PR or MR. If no other work on the software is currently in progress or planned for the immediate future, the PR or MR author should release a new version after merging the PR or MR.
Releasing should happen automatically when pushing a Git tag, for example this can be implemented as a CI job which is triggered by pushing the tag.
Assigning a proper version to a release helps to understand it.
We follow the semantic versioning scheme with a
v prefix. (E.g.
See our Handbook for some Git guidelines.
To ensure good code quality, we automate test code execution. These tests must run in a CI pipeline (Either GitHub Action or GitLab CI).
We try to have a reasonably high test coverage. Enough to give us competence when upgrading libraries, fixing bugs and implementing new features. While 100% coverage can be a good idea for some packages and functions we don’t expect it anywhere. We keep a balance between coverage, test complexity, and engineering time.
A good start for tests is a happy-path test and tests for special error handling second.
To make our code look familiar throughout the company, we provide repository templates which are maintained:
For Go: github.com/vshn/go-bootstrap
For Commodore Components: github.com/projectsyn/commodore-component-template
We store our Git repositories in two locations:
All code which is meant to be public (no private repos on GitHub)
All code which isn’t meant to be public is stored on git.vshn.net
Whenever possible we strive to have our code on GitHub.
There is a high risk that we become victims of the Not Invented Here (NIH) syndrome: the tendency to avoid external or third party solutions in favor of building our own.
Our goal is to avoid this wherever feasible, and we assess all our architectural decisions accordingly. There are cases where building something new is appropriate, but we should always prefer gluing together existing things if the result is good enough.
We always keep maintainability in mind. We’d rather have our own tool where we are pretty confident upgrading it than multiple complex systems that we can’t reasonably test before upgrades.
If we’re working on a feature or fix and come across a suboptimal or overly complicated bit of code, we address the issue, if it’s doable with reasonable effort (<2h). Otherwise, we create an issue and mark it as "Technical Debt" in Jira. We don’t mix such changes with the MR or PR relating to the original feature or fix, but create a separate PR addressing the issue.