GitHub, Pull Requests and Your Code in Checkmk

Right before the Checkmk Conference #9 it seems fitting to look back on the past year. One of the big wins in our cooperation with the community, in my humble opinion, was the improvements of our Pull Request Review process on GitHub.

Every day, in order to keep Checkmk up to date with the needs of our users, the development team (aka the maintainers of Checkmk) have to prioritize some tasks over the others, including creating new and updating the existing checks, improving security, designing new features, fixing bugs and releasing new versions. They also have to find time to communicate with the contributors, reviewing pull requests on GitHub and commenting on what could be improved.

Before May 2022, it was not easy for the team to keep up with all the contributions, which led to delays in reviews and misunderstandings. Even though we appreciated people’s willingness to help, doing it right required a systematic change. Here is what we did.

First of all, we set up a dedicated process, so code contributions could receive the care and attention they deserve. We created refined rules that help us decide on whether the Pull Request should be assigned to developers for implementation or declined, for example, in the event that the PR does not follow the contribution guidelines.

We established a system where we pay more systematic attention to pull requests and make decisions on closing requests. This happens mainly if the changes do not adhere to the standards Checkmk code has or because a certain change does not feel like the right one to make at the moment. The changes must also adhere to updated guidelines. With all these changes, we managed to reduce the request processing time to several weeks or less. At the moment of writing this blog, I can only see 20 open requests. As part of the team of maintainers behind Checkmk, I can say that we are satisfied with the progress we had last year. But we always strive for improvement, and here is how you – as a code contributor– could help us. 

Here are a couple of details of the new process, which you can follow to increase the chances of your PR being accepted. Some of these insights might also help to understand why some change has not been approved even though it follows the rules perfectly and could bring value to the project. 

As said before, one of the biggest criteria for approving a pull request is that it follows the contribution guidelines. There are several reasons why it is so important:

First, the style guides. Any source code should be readable (to people who speak the language, that is) and neat, so whenever that part of code needs to be updated or changes completely, everyone can understand it. It is like with books – a consistent layout is the way to go if you want to enjoy the reading – or keep writing. That is why only the contributions following the style guide will be accepted. 

Another thing is the process we use for committing changes and testing them through CI. As with the style guides, commit messages should be clear and they should be submitted through the correct git procedure – the team put a lot of effort into writing down all the steps, so it should be the easy part. Testing is essential – if the change does not pass the tests, it means that it is not functioning correctly. In this case, please fix the code if you want it to be approved.

“Divide and conquer” – this motto not only works in politics but also in pull requests. Precise short changes tackling a very specific issue or a part thereof are the easiest for review – which means the process will take less time. Another reason is that the more you divide a big idea into smaller changes, the better the chance that at least some of these smaller changes will be approved.

The team simply cannot allocate enough resources to review long requests – a good comprehensive review takes time and focus, which is very difficult to maintain with complex changes within one request. Another argument for separate changes in separate PRs is that each PR will have a better context in the description – which allows the reviewer to better understand your idea better. So please, try to stay precise, give us the context and by doing so, help make us able to validate your contribution and more likely to accept it.

Some changes – for example fixes or refactorings – really do require changing the source code and going through the pull request review process. But for many features, uploading them to the Checkmk Exchange might be a better option. Why? The review process is less complicated; you will be able to continue changing it by adding new versions of the package, and you will definitely receive direct appreciation from the community as you retain the ownership of your package that way.

Sometimes we receive great contributions but still have to reject them for a reason that might not seem that obvious. Some changes might work in integration with some very specific tool that we do not have on our hands on, and so we cannot validate a change like that. For example, if you create a pull request that implements an integration with some exotic Oracle instance that we do not have access to, we cannot test it – even if you can, we cannot. And as a team that not only maintains Checkmk but also bears the responsibility for the project, we cannot in good conscience accept such a change even if we believe it to be useful. In cases such as this we would, once again, recommend uploading your change as an Exchange package.

Another case is if you test your change and it works for you but does not work for others. We will not be able to accept such changes, as we need to consider the project and all of its users as a whole.

Please note that the more the developer team does, the more contributions we have – and the more pull requests we receive. That is why it is very important to follow the discussion with the representatives of the development team in the PR comments, as well as be patient if it takes a little more time for us to review your request. We care about and appreciate each contribution, which is why we want to give it the time it deserves to make sure it turns into an organic part of Checkmk.

We hope that the changes and more transparent rules for pull requests will allow us to work together more – allowing you to more conveniently submit changes and letting us check and accept them. In the graph below, you can see how our rate of responsiveness to pull requests has increased over the last year – and we believe we can do even better as we go on. We hope to see you with us on that journey.