Niklas Lochschmidt

Guideline for Developing Software

In a recent project I took some time to write down a guideline for the development team on how we want to develop software. The idea has been that the whole team can agree and commit to the guideline, defend it if need be and use it to showcase our development values and processes to new developers. Things turned out a bit different, but nevertheless I think it may contain some value to others. I am very much looking forward to suggestions, so leave a comment below or on Twitter. Without further ado:

Our Guideline for Developing Software

This guide is intended to be read in brain-on mode. If things don’t make sense you are responsible for raising them to your peers.

Our common goal is to deliver value to our customer and the users of our services. We aim to keep our processes to a minimum and provide the fastest route possible from an idea to creating value. To ensure that we stay fast, we need to write high-quality code in the sense that it works in production, is robust and stays flexible and maintainable in the long run.

We aim to achieve this goal through the following key practices:

Clear language

The code is the truth, because the code is what gets executed. Therefore the code should be clear and expressive. This is especially important in the core domain logic where the entities and actions should be named according to the language of the business. PersonFactory.create(…) is much less expressive than Onboarding.registerCustomer(…).

Use consistent naming for the entities in the system. Do not say customer and user in the same context unless both concepts are indeed different. Refer to the glossary in case of doubt.

Only write comments in code as a last resort when the comments convey additional information that can not already be derived from the code. Always try to make the intent clear by refactoring names of variables, classes and methods or restructure the code to be simpler to reason about. When you really need to write comments, make sure to not only write what the code does, but also why you decided to do it in that way.

Make sure to write good commit messages. A short summary line with at most 50 characters followed by a blank line and the main body of the commit message with 72 characters per line. This ensures that the subject line is never truncated in notifications and summary emails and is well formed for use with git log -S and the IDEs annotation features.

When writing your commit message, think about future-you or a fellow developer looking at your commit messages on a particular line of code and try to answer the questions: Why was this introduced/changed, how does it solve the problem and whether or not there are any known shortcomings of the solution.

Independent of the programming language, we try to adhere as close as possible to the default code style of that language. However, we always allow a limit of 120 characters per line (excluding special cases like import statements which are allowed to be longer). This character limit ensures, that we can use longer names on a single line, but still show all code in a vertical split on a regular wide-screen display.

Testing

Writing tests first is strongly encouraged. All business logic needs to be covered by unit-tests to the degree that the developer as well as the reviewer have enough confidence that the code will work in production as intended.

Apart from the business logic, testing is still encouraged but not strictly mandatory, as long as the code in question is covered by larger tests, like acceptance and integration tests. For us an acceptance test is testing a module as a black-box while an integration test is also testing with real external dependencies (e.g. the database).

Testing of third party libraries and frameworks is only necessary, when we have to use an unstable version (don’t do that), where we suspect that errors can be made easily or where we have experienced actual issues and need to discover regressions early.

Code Review

By default all code written and checked in needs to be reviewed by at least one peer. In addition this should happen before the code is pushed to the master-branch. The reason for this is not to control one another, but to detect defects and spread knowledge about the code throughout the team. It is primarily a possibility to learn from one another.

It is therefore imperative that code reviews are conducted with respect for one another. Destructive comments, insults, blaming and generally rude comments are simply unacceptable. In case you are on the receiving end of such a comment, either as developer or reviewer, get another peers opinion or escalate the issue. Also go and read the excellent blog post by Philip Hauer on Code Review Guidelines)

Small changes, like changes on configuration files, on build steps, simple version bumps of dependencies, fixing typos, adding clarifying comments, code-style cleanups etc. do not need to be peer reviewed. Some of those tasks might even be automated and our extensive tests should be able to detect bugs and incompatibilities introduced like that.

We offer two ways to perform a code review:

Pull Requests

As developer, you start your work by creating a branch from master for the task you are about to start. Create the branch from the GitLab issue page, so that all commits belonging to the issue are tracked. Once you finished the task, you should optionally squash the commits and rebase again on master. Then create a pull request and assign someone else to review your code.

As reviewer, once you get assigned to a pull request, try to review the code as soon as possible. Leave constructive remarks as comments on the commits in GitLab, make sure to point out code that is hard to understand and if possible provide small snippets of improved code. Also make sure the tests are correct and sufficient. Also indicate which parts must be changed and which are nice to have. In case there are no critical issues, feel free to merge the pull request into master, otherwise notify the initial developer.

As developer, fix the issues that came up during code review and decide whether the code needs a second review. Otherwise merge the pull request into master.

Code Walk-Through

This is reserved for cases, where a task has become too large to be reviewed alone. The decision to do a walk-through can come from the developer or from a reviewer starting to review a pull request.

The developer should guide the reviewer through the finished code without necessarily looking at individual commits. Ideally the reviewer should have control over the keyboard in order to be able to easily inspect other related pieces of code and quickly jump back and forth in the code base. Discoveries during the walk-through needs to be captured and corrected by the developer afterwards.

Once the developer has incorporated the feedback from the walk-through the pull requests gets updated and a normal code review of the latest changes should be performed.

Bonus: Pair Programming

Code that has been written as part of a pairing session can be merged directly into master without a formal code review.

Continuous Integration

We aim to integrate everyone’s code into master as often as possible, so that we get very close to everyone working directly on the master branch, because everything on the master branch is on track for delivery to the customer.

For this to work we need to aim for right-sized increments that can be developed and reviewed in a day. If a task is too large, try splitting it into sub-tasks or create daily pull requests of intermediate functionality, that can already be reviewed and merged.

By continuously merging into master we acknowledge that our brains working memory is limited and therefore recognize that parallel work in progress for any developer needs to be minimized. The work in progress for a single developer should ideally be a single task. In case a developer has open review requests, those take precedence over picking a new task from the backlog.

All commits get build and tested automatically on the CI build server and an artifact (e.g. a binary, JAR or a Docker image) is created and stored away. In case of the master this artifact gets deployed into a staging environment. Build artifacts resulting from a successful build of the master-branch must always be ready to be deployed into production. That means a red build is reason for immediate investigation and recovery. Also unstable (flaky) builds must be fixed asap.

If you are working on a feature that should not be active yet, use feature toggles to hide it until it’s ready.

Shared Code Ownership

Since our common goal is to create value for the customer and the code is just the vehicle to get there, we must not get emotionally attached to the code. If code has been written by someone else than yourself, you have full permission to change it without consulting with the original developer, provided you test the code and let it be reviewed as always.

Always assume that your code and everyone else’s code has always been written with the best of intentions, to the best of their abilities and based on the full knowledge at that time. When you receive feedback in a code review on some code you wrote, it will not be a criticism on you as a person.

That’s it, now let’s go!

P.S. I am aware that there is nothing inherently new in this text. It is simply a selection of valued practices that can be found in the Software Crafter movement, Domain Driven Design (DDD), Test Driven Development (TDD), Extreme Programming (XP), DevOps and Agile