I review a lot of pull requests for Helm, the package manager for Kubernetes. While reviewing, I’ve noticed some common pieces of advice or guidance I give along the way. This post documents some of those.
Don’t Change APIs
In minor and patch releases of Helm we don’t change APIs. Helm follows semantic versioning and we have documented our backwards compatibility. This is both for end users of Helm and for maintainers of Helm to refer to.
The one exception to this rule is for experiments. The only current experiment is the OCI support.
So, when a pull request changes a public Go function signature it will automatically be flagged and can’t be merged. This will break users of Helm as a package, something many are doing.
The work around for this is to create a new public function, keep the old one, and mark it as deprecated or that the two need to be merged in a future major version. There are examples of this in the code already.
Describe Your Use Case And How It Impacts Others
Helm pull requests should be tied to an issue. There are only a few exceptions to this, such as when a dependency is updated. Major feature additions may even require a Helm Improvement Proposal.
Helm is stable software used by many people. We need to keep the stability while still moving forward. That means, we take change management seriously. Part of doing that is to consider the change, who it impacts, and how it changes the experience of others.
For example, consider the role the change impacts. On Helm we have documented the roles that are supported and not supported. They are even prioritized.
It makes it much easier when the use case and impacts are taken into account.
Make Sure Your Change Is In Scope
One of the reasons we want an issue or HIP filed before a pull request is created for a new feature has to do with the scope of Helm. Helm is a package manager like apt, yum, zypper, etc. It’s not a configuration manager like Ansible or Chef. It can be used with one, though.
Configuration management can happen with other tools that use Helm. Tools like Helmfile, Fleet, and the Flux Helm Operator (to name a few). Helm has its scope and those tools have theirs.
Some ideas for Helm to change are out of scope. Before writing a bunch of code, it can be useful to see if it’s a good idea to get started. Or, not.
Multiple Small Pull Requests Are Better Than Large Ones
Maintainers are busy. They work on other things in addition to Helm. When they carve out time to review pull requests, the large ones are always going to take more time and work. Sometimes this cannot be avoided. But, most of the time it can.
When someone has written a change that has thousands of lines changed, I see someone who put in a lot of effort to making the change. After putting in all that effort they submit the pull request and then wait for feedback.
On the flip side, maintainers see a change like that and see a lot of work they have to put in on the review. They have to make sure it fits for Helm, is backwards compatible, and has a good code quality.
It’s not always possible but, the more concise the change the easier it is to have it reviewed and merged.
Tests!
When folks submit changes they often focus on the bug fix or the new feature. If there are not tests a maintainer will often ask for them right away. We want tests. They make sure the feature works as expected and regressions don’t happen in the future.