|
| 1 | +--- |
| 2 | +title: Helm Core Maintainer's Onboarding Guide |
| 3 | +sidebar_label: Maintainer Onboarding Guide |
| 4 | +--- |
| 5 | + |
| 6 | +<!-- |
| 7 | +THIS FILE IS AUTOMATICALLY IMPORTED FROM THE HELM/COMMUNITY REPOSITORY |
| 8 | +DO NOT EDIT THIS FILE DIRECTLY - IT WILL BE OVERWRITTEN |
| 9 | +
|
| 10 | +TO MAKE CHANGES: |
| 11 | +- Edit the source file at: https://github.com/helm/community/blob/main/helm-maintainers-onboarding-guide.md |
| 12 | +--> |
| 13 | + |
| 14 | +Welcome, new core maintainer! Now that you have been voted in as a new Helm maintainer, we wanted to make sure we got you off to a good start. This guide is an introduction to life as a Helm maintainer. |
| 15 | + |
| 16 | +## Your Core Responsibilities |
| 17 | + |
| 18 | +The primary objective of a core maintainer is to _further Helm's goal of being both an outstanding package manager and a thriving and friendly community_. |
| 19 | + |
| 20 | +All Helm maintainers are expected to do the following: |
| 21 | + |
| 22 | +- Positively represent Helm |
| 23 | +- Hang out in the #helm-dev and #helm-users Slack channel |
| 24 | +- Make a good-faith effort to come to the public developer's meeting |
| 25 | +- Participate in formal Helm decision making, such as casting an official vote for new maintainers |
| 26 | +- Uphold the [CNCF Code of Conduct](https://github.com/cncf/foundation/blob/master/code-of-conduct.md) (that is, not just follow, but cultivate that as part of the project's culture) |
| 27 | +- Participate in resolution of critical security issues as needed |
| 28 | + |
| 29 | +## You're Encouraged To… |
| 30 | + |
| 31 | +A successful team is not a homogenous team. Each contributor brings unique strengths and talents to the team. Because of that, we don't include things like coding PRs in the list of core responsibilities. You know your own strengths, but here are some suggested ways you can further contribute as a core maintainer: |
| 32 | + |
| 33 | +- Triage the issue queue: This is the tactful process of responding to often-frustrated users who file bugs or request features. We take turns "owning" this responsibility week to week. |
| 34 | +- Slack champion: Champions in Slack are maintainers who proactively work with users in #helm-users. This is quite possibly the most important way we convey Helm's friendly culture. |
| 35 | +- Review PRs: This is the process of reading code from others, offering constructive and friendly guidance when necessary, and ultimately deciding whether it meets the project requirements. |
| 36 | +- Release planning: As described in [HIP 0002](hips/hip-0002.md), we endeavor to plan ahead and plan specifics. This is the process of planning each release, and then assigning issues to that release. |
| 37 | + |
| 38 | +## If You See Code of Conduct Violations or Bad Actors… |
| 39 | + |
| 40 | +- A Code of Conduct violation is a case where someone appears to have transgressed the CNCF Code of Conduct. |
| 41 | +- A Bad Actor is someone who is causing harm to the project (intentionally or unintentionally) through their words or actions. |
| 42 | + |
| 43 | +If you feel like you are up to it, the best first response to either is an attempt at gentle redirection. If you do not feel like you are up for that, contact one or more of the other core maintainers and let them know about the situation. Collectively, we must make it an obligation to protect the community, and enforce standards of conduct. |
| 44 | + |
| 45 | +If a gentle redirection is not sufficient, code of conduct violations need to be reported through the proper CNCF channels. The process of reporting these is documented in the code of conduct. Unless there are mitigating circumstances, Code of conduct violations reported upstream should be shared with the other Helm maintainers so they are prepared to handle the issue as it pertains to Helm while action is handled elsewhere. |
| 46 | + |
| 47 | +## What SemVer Means to Us |
| 48 | + |
| 49 | +If you are not familiar with SemVer (the standard), [give it a quick read](http://semver.org). We follow SemVer with a high degree of rigor. This often means that we have to tell people "no" or "wait" in order to preserve backward compatibility. |
| 50 | + |
| 51 | +The rough application of our SemVer application goes like this: For any non-major release... |
| 52 | + |
| 53 | +Charts: |
| 54 | + |
| 55 | +- The Chart format MUST NOT change. |
| 56 | +- Optional fields may be added to Chart.yaml, requirements.yaml, but chart fields MUST NOT be deleted or modified. |
| 57 | +- Mandatory fields MUST NOT be added to Chart.yaml or requirements.yaml |
| 58 | +- New template functions may be added, but existing functions MUST NOT be deleted. |
| 59 | +- The default output of any command/subcommand SHOULD NOT change, except to fix bugs. |
| 60 | +- Any modifications to chart internals MUST be backwards compatible with charts produced from Helm 2.0.0 onward. |
| 61 | + |
| 62 | +Command Line: |
| 63 | + |
| 64 | +- Subcommands MUST NOT be removed |
| 65 | +- Subcommands SHOULD NOT change in meaning |
| 66 | +- Command line flags MUST NOT be deleted |
| 67 | +- Command line flags SHOULD NOT change in meaning |
| 68 | + |
| 69 | +Code: |
| 70 | + |
| 71 | +- The Protobuf files can have optional fields added, but fields MUST NOT be deleted or modified. |
| 72 | +- Protobuf files MUST NOT have mandatory fields added |
| 73 | +- The public API of anything in `pkg/` SHOULD NOT be modified |
| 74 | +- Go interfaces in `pkg/` MUST NOT be modified |
| 75 | +- The first release of a major feature addition SHOULD be hidden behind an `experimental` feature flag. (See [Enabling OCI Support](/docs/topics/registries#enabling-oci-support) and [add feature gates](https://github.com/helm/helm/pull/6185) for an example.) |
| 76 | +- Critical security fixes MAY be sufficient cause to ignore the SemVer rules (but we try not to do that). |
| 77 | + |
| 78 | +Compatibility with Kubernetes: |
| 79 | + |
| 80 | +- A version of Helm MUST be compatible with the version of Kubernetes that it is developed against |
| 81 | +- A version of Helm SHOULD be compatible with the previous two Kubernetes releases |
| 82 | +- A version of Helm SHOULD be forward-compatible to the greatest extent possible |
| 83 | + |
| 84 | +(The velocity and frequency of major changes in Kubernetes precludes us from making strong compatibility guarantees; see the [Helm Version Support Policy](/docs/topics/version_skew) for specific supported versions.) |
| 85 | + |
| 86 | +During a major release, all code is subject to revision, but Chart backward compatibility SHOULD be retained. |
| 87 | + |
| 88 | +## Nuts and Bolts |
| 89 | + |
| 90 | +This section explains (in more detail) how we do some things in Helm |
| 91 | + |
| 92 | +### Slack and Issue Interactions |
| 93 | + |
| 94 | +- We are friendly and empathetic. |
| 95 | +- We understand that users are often frustrated by the time they seek our help. |
| 96 | +- We make a good-faith effort to help where possible. |
| 97 | +- We sometimes have to try to pull other people into a discussion to get the right answer, but we don't _demand_ that those people participate. |
| 98 | +- Sometimes Helm cannot (and will not) fix a user's problem. We are okay admitting that. |
| 99 | + |
| 100 | +### Issue Triaging |
| 101 | + |
| 102 | +Core maintainers take turns triaging the issue queue. The responsibilities of a triaging run are: |
| 103 | + |
| 104 | +- Tag all new issues |
| 105 | + - Bugs are tagged `bug` |
| 106 | + - Feature requests are tagged `feature` |
| 107 | + - The "default" is to tag an issue as a `question/support` |
| 108 | + - Anything having to do with docs are tagged `docs` and such issues may be transferred to `helm/helm-www` |
| 109 | + - If the fix is simple (<10 lines of code), tag it `good first issue` |
| 110 | + - If a feature is deemed a Good Idea (TM), but not something we're likely to do in the near future, label it `help wanted` |
| 111 | +- Question/Support |
| 112 | + - This basically means that we expect ongoing discussion |
| 113 | + - Sometimes we flag features and proposals as support if they seem to hinge on a misunderstanding or far-out idea |
| 114 | + - Sometimes we start "bugs" as "question/support" until we get sufficient info to figure out what is going on |
| 115 | +- Bugs: |
| 116 | + - If it's high severity, add it to the next milestone (patch or feature) |
| 117 | + - Otherwise, add it to the next MINOR release |
| 118 | +- Features: |
| 119 | + - If it violates the SemVer rules, open a dialog on whether there is an alternate way to do this that will not break our SemVer rules |
| 120 | + - If it is a major feature (new subcommand, new way of doing something), ask if it can be implemented as a plugin or extension |
| 121 | + - All features and breaking changes MUST be described in a [HIP](hips/hip-0001.md), and that HIP must pass the usual process. (See HIP 12: "[Helm 4 Development Process](hips/hip-0012.md)"). |
| 122 | + |
| 123 | +For information on labels and their meaning, see [CONTRIBUTING.md](https://github.com/helm/helm/blob/main/CONTRIBUTING.md#labels) for the project. |
| 124 | + |
| 125 | +During your week of triaging, periodically scan back through known recent issues to see what has been updated. |
| 126 | + |
| 127 | +Finally, question/support, feature, proposal and non-updated pull requests _may_ be closed after 30 days of inactivity. It is up to your judgment on whether or not you close one. The Helm project's [stale issue bot](https://github.com/helm/helm/blob/main/.github/workflows/stale-issue-bot.yaml) marks an issue as stale if it has been open for 90 days with no activity, and closes it in an additional 30 days if no further activity occurs. |
| 128 | + |
| 129 | +To flag issues that should not be closed automatically, use the labels `keep open` or (if appropriate) `v4.x`. |
| 130 | + |
| 131 | +### PR Reviews |
| 132 | + |
| 133 | +Helm is a community-driven project. We desire community-contributed code. The role of the core maintainer, in regards to the code, is to ensure that new PRs _stay true to Helm's intent, solve real problems for real users, and meet our quality guidelines_. |
| 134 | + |
| 135 | +- Helm's Intent: Make it easy to package, share, deploy, and manage Kubernetes applications. Our guiding user paradigm is "the package manager for Kubernetes". |
| 136 | +- Solve Real Problems for Real Users: We only want to add features to Helm if they are of benefit to many users and for non-niche use cases. This sometimes means accepting the fact that there are some things that Helm cannot do. |
| 137 | + - Helm is not a testbed for new ideas. We push back on cases where users say "I developed a new technology, and I want Helm to support it." |
| 138 | + - Many times, the way to address PRs that are niche is to suggest that they be made into plugins or extensions |
| 139 | +- Quality Guidelines: _We are not perfectionists. But we are enthusiasts for clarity, maintainability, and consistency._ |
| 140 | + |
| 141 | +When reviewing PRs, we need to be actively encouraging to the submitter. Strive to not leave a PR review with only "fix-it" comments. Add at least one positive note (and saying "thanks for the PR" does not count). |
| 142 | + |
| 143 | +#### Details on How We Review PRs |
| 144 | + |
| 145 | +If a PR passes the "Intent" and "Solves Problems" criteria above, we view our goal as getting the PR into shape and merged with minimal frustration to the submitter. |
| 146 | + |
| 147 | +**SemVer Constraints:** Make sure a PR does not violate the SemVer constraints as explained above. This includes making sure API, ProtoBuf/gRPC, flags, commands, and formats remain compatible. Submitters often do not realize that their changes would break compatibility, so be extra gentle when explaining this. |
| 148 | + |
| 149 | +**Coding Conventions/Idioms:** Our points of reference are [Effective Go](https://golang.org/doc/effective_go) and the [Go Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments) in the Go Wiki. We follow Effective Go closely, and take the Go Code Review Comments as "decent guidelines". |
| 150 | + |
| 151 | +We tend to avoid comments of the form "This works fine, but another way of doing it is..." unless the other way of doing it conveys clear advantage over the existing way. |
| 152 | + |
| 153 | +**Code Style:** We have automated our style reviews. If the `golangci-lint` rules in `make test-style` pass, we tend to not place any additional requirements on the user. |
| 154 | + |
| 155 | +**Testing:** We strongly encourage PR submitters to include unit tests if they add new code. A PR should be _blocked_ if it introduces substantial new code, but that code is not covered by unit tests. The exception to this rule is if the package into which the PR is contributed is deemed "very difficult to test" by the core maintainers. (We do have a few packages like this.) |
| 156 | + |
| 157 | +**Documentation:** If a PR contains a new feature, we tend to require accompanying documentation. This may come in the form of additional help text in the `helm --help` section, or it may require additional edits or updates in the docs. Note that we need to be sensitive to those who do not speak English as a first language. It is okay to drop the requirement when the PR submitter does not feel they can produce quality written help. |
| 158 | + |
| 159 | +**Scope:** We try hard to limit our comments to the things the submitter has changed. To that end, we try not to require them to make changes that are not directly related to their fix. |
| 160 | + |
| 161 | +### Details on Merging PRs |
| 162 | + |
| 163 | +A PR can only be merged only if: |
| 164 | + |
| 165 | +- All tests are passing for that PR (Circle is green) |
| 166 | +- The PR has one LGTM (small, medium) or two LGTMs (large). The GitHub Review tool is used to LGTM. |
| 167 | +- The PR's milestone is set to the current Minor or Patch release. Ensure that in the course of Minor and Patch release work, you never merge PRs labeled with the next Major version. For example, while v3 is the current Major version, we do not merge PRs labeled with [v4.x](https://github.com/helm/helm/pulls?q=is%3Aopen+is%3Apr+label%3Av4.x). |
| 168 | +- The DCO bot has passed for the PR. |
| 169 | + |
| 170 | +Miscellaneous rules: |
| 171 | + |
| 172 | +- If a core maintainer opened a PR, that person must be the one to either (a) merge the PR, or (b) ask another core maintainer to merge on their behalf. |
| 173 | + |
| 174 | +### The Release Process |
| 175 | + |
| 176 | +Corresponding to SemVer, we have three different types of release: |
| 177 | + |
| 178 | +- Major: 1.0.0, 2.0.0, etc. Breaks compatibility with previous releases. |
| 179 | +- Minor: 2.1.0, 2.2.0, etc. Maintains compatibility per our SemVer rules above, but may add new features, fix bugs. |
| 180 | +- Patch: 2.1.1, 2.1.2, etc. Maintains compatibility, adds no features, but fixes bugs. |
| 181 | + |
| 182 | +Major releases and Minor releases are done by tagging `main` with the version number, and then running the release scripts. |
| 183 | + |
| 184 | +Patch versions are done by a more complex process. They start from the `release-X.Y` branch and cherry-pick from `main`. A release branch is forked from the last minor release, and is then maintained in parallel with the main: |
| 185 | + |
| 186 | +``` |
| 187 | +---- v2.3.0 ---- Fix #1 --- Feature #2 --- Fix #3 --- ... [main] |
| 188 | + \ \ \ |
| 189 | + release-2.3 ---- Fix #1 ------------- Fix #3 --- v2.3.1 |
| 190 | +``` |
| 191 | + |
| 192 | +Only fixes are pulled from `main` onto the `release-X.Y` branch. Features are not. |
| 193 | + |
| 194 | +#### Cherry-Picking |
| 195 | + |
| 196 | +The process of cherry-picking is done by the maintainers, typically without bringing the fix contributor back into the discussion. When a cherry-pick causes a merge conflict, the one doing the cherry-picking must decide whether to resolve the cherry-pick or whether to skip merging that issue into the release-X.Y branch. |
| 197 | + |
| 198 | +The best practice for cherry-picking is: |
| 199 | + |
| 200 | +1. Checkout the `release-X.Y` branch |
| 201 | +1. Pick a commit into the branch |
| 202 | +1. If conflict, resolve the conflict |
| 203 | +1. Run `make test` |
| 204 | +1. Repeat 2-4 for each necessary fix |
| 205 | +1. Push the release-X.Y branch to your own clone |
| 206 | +1. Create a pull request from your clone against the upstream `release-X.Y` |
| 207 | +1. Ask other core maintainer(s) for review |
0 commit comments