msb-public
/
helm
mirror of https://github.com/helm/helm
1
0
Code Issues Projects Releases Wiki Activity

docs: bring CONTRIBUTING doc up-to-date with current practices (#5401)

Browse Source 
Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>
pull/5413/head
Matthew Fisher 7 years ago committed by GitHub
parent f4737d19aa
commit e4892e7575
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 91 additions and 92 deletions
Show Stats Download Patch File Download Diff File

117
CONTRIBUTING.md
Unescape View File

@ -43,10 +43,10 @@ used for helping us keep work organized: `Upcoming - Minor` and `Upcoming - Majo
`Upcoming - Minor` is used for keeping track of issues that aren't assigned to a specific `Upcoming - Minor` is used for keeping track of issues that aren't assigned to a specific
release but could easily be addressed in a minor release. `Upcoming - Major` keeps track release but could easily be addressed in a minor release. `Upcoming - Major` keeps track
of issues that will need to be addressed in a major release. For example, if the current of issues that will need to be addressed in a major release. For example, if the current
version is `2.2.0` an issue/PR could fall in to one of 4 different active milestones: version is `3.2.0` an issue/PR could fall in to one of 4 different active milestones:
`2.2.1`, `2.3.0`, `Upcoming - Minor`, or `Upcoming - Major`. If an issue pertains to a `3.2.1`, `3.3.0`, `Upcoming - Minor`, or `Upcoming - Major`. If an issue pertains to a
specific upcoming bug or minor release, it would go into `2.2.1` or `2.3.0`. If the issue/PR specific upcoming bug or minor release, it would go into `3.2.1` or `3.3.0`. If the issue/PR
does not have a specific milestone yet, but it is likely that it will land in a `2.X` release, does not have a specific milestone yet, but it is likely that it will land in a `3.X` release,
it should go into `Upcoming - Minor`. If the issue/PR is a large functionality add or change it should go into `Upcoming - Minor`. If the issue/PR is a large functionality add or change
and/or it breaks compatibility, then it should be added to the `Upcoming - Major` milestone. and/or it breaks compatibility, then it should be added to the `Upcoming - Major` milestone.
An issue that we are not sure we will be doing will not be added to any milestone. An issue that we are not sure we will be doing will not be added to any milestone.
@ -55,18 +55,17 @@ A milestone (and hence release) is considered done when all outstanding issues/P
## Semver ## Semver
Helm maintains a strong commitment to backward compatibility. All of our changes to protocols and formats are backward compatible from Helm 2.0 until Helm 3.0. No features, flags, or commands are removed or substantially modified (other than bug fixes). Helm maintains a strong commitment to backward compatibility. All of our changes to protocols and formats are backward compatible from Helm 3.0 until Helm 4.0. No features, flags, or commands are removed or substantially modified (other than bug fixes).
We also try very hard to not change publicly accessible Go library definitions inside of the `pkg/` directory of our source code. We also try very hard to not change publicly accessible Go library definitions inside of the `pkg/` directory of our source code.
For a quick summary of our backward compatibility guidelines for releases between 2.0 and 3.0: For a quick summary of our backward compatibility guidelines for releases between 3.0 and 4.0:
- Protobuf and gRPC changes MUST be backward compatible.
- Command line commands, flags, and arguments MUST be backward compatible - Command line commands, flags, and arguments MUST be backward compatible
- File formats (such as Chart.yaml, repositories.yaml, and requirements.yaml) MUST be backward compatible - File formats (such as Chart.yaml) MUST be backward compatible
- Any chart that worked on a previous version of Helm MUST work on a new version of Helm (barring the cases where (a) Kubernetes itself changed, and (b) the chart worked because it exploited a bug) - Any chart that worked on a previous version of Helm 3 MUST work on a new version of Helm 3 (barring the cases where (a) Kubernetes itself changed, and (b) the chart worked because it exploited a bug)
- Chart repository functionality MUST be backward compatible - Chart repository functionality MUST be backward compatible
- Go libraries inside of `pkg/` SHOULD remain backward compatible (though code inside of `cmd/` may be changed from release to release without notice). - Go libraries inside of `pkg/` SHOULD remain backward compatible, though code inside of `cmd/` and `internal/` may be changed from release to release without notice.
## Issues ## Issues
@ -74,42 +73,45 @@ Issues are used as the primary method for tracking anything to do with the Helm
### Issue Types ### Issue Types
There are 4 types of issues (each with their own corresponding [label](#labels)): There are 5 types of issues (each with their own corresponding [label](#labels)):
- Question: These are support or functionality inquiries that we want to have a record of for
- `question/support`: These are support or functionality inquiries that we want to have a record of for
future reference. Generally these are questions that are too complex or large to store in the future reference. Generally these are questions that are too complex or large to store in the
Slack channel or have particular interest to the community as a whole. Depending on the discussion, Slack channel or have particular interest to the community as a whole. Depending on the discussion,
these can turn into "Feature" or "Bug" issues. these can turn into `feature` or `bug` issues.
- Proposal: Used for items (like this one) that propose a new ideas or functionality that require - `proposal`: Used for items (like this one) that propose a new ideas or functionality that require
a larger community discussion. This allows for feedback from others in the community before a a larger community discussion. This allows for feedback from others in the community before a
feature is actually developed. This is not needed for small additions. Final word on whether or feature is actually developed. This is not needed for small additions. Final word on whether or
not a feature needs a proposal is up to the core maintainers. All issues that are proposals should not a feature needs a proposal is up to the core maintainers. All issues that are proposals should
both have a label and an issue title of "Proposal: [the rest of the title]." A proposal can become both have a label and an issue title of "Proposal: [the rest of the title]." A proposal can become
a "Feature" and does not require a milestone. a `feature` and does not require a milestone.
- Features: These track specific feature requests and ideas until they are complete. They can evolve - `feature`: These track specific feature requests and ideas until they are complete. They can evolve
from a "Proposal" or can be submitted individually depending on the size. from a `proposal` or can be submitted individually depending on the size.
- Bugs: These track bugs with the code or problems with the documentation (i.e. missing or incomplete) - `bug`: These track bugs with the code
- `docs`: These track problems with the documentation (i.e. missing or incomplete)
### Issue Lifecycle ### Issue Lifecycle
The issue lifecycle is mainly driven by the core maintainers, but is good information for those The issue lifecycle is mainly driven by the core maintainers, but is good information for those
contributing to Helm. All issue types follow the same general lifecycle. Differences are noted below. contributing to Helm. All issue types follow the same general lifecycle. Differences are noted below.
1. Issue creation 1. Issue creation
2. Triage 2. Triage
- The maintainer in charge of triaging will apply the proper labels for the issue. This - The maintainer in charge of triaging will apply the proper labels for the issue. This
includes labels for priority, type, and metadata (such as "starter"). The only issue includes labels for priority, type, and metadata (such as "good first issue"). The only issue
priority we will be tracking is whether or not the issue is "critical." If additional priority we will be tracking is whether or not the issue is "critical." If additional
levels are needed in the future, we will add them. levels are needed in the future, we will add them.
- (If needed) Clean up the title to succinctly and clearly state the issue. Also ensure - (If needed) Clean up the title to succinctly and clearly state the issue. Also ensure
that proposals are prefaced with "Proposal". that proposals are prefaced with "Proposal: [the rest of the title]".
- Add the issue to the correct milestone. If any questions come up, don't worry about - Add the issue to the correct milestone. If any questions come up, don't worry about
adding the issue to a milestone until the questions are answered. adding the issue to a milestone until the questions are answered.
- We attempt to do this process at least once per work day. - We attempt to do this process at least once per work day.
3. Discussion 3. Discussion
- "Feature" and "Bug" issues should be connected to the PR that resolves it. - issues that are labeled as `feature` or `bug` should be connected to the PR that resolves it.
- Whoever is working on a "Feature" or "Bug" issue (whether a maintainer or someone from - Whoever is working on a `feature` or `bug` issue (whether a maintainer or someone from
the community), should either assign the issue to them self or make a comment in the issue the community), should either assign the issue to themself or make a comment in the issue
saying that they are taking it. saying that they are taking it.
- "Proposal" and "Question" issues should stay open until resolved or if they have not been - `proposal` and `support/question` issues should stay open until resolved or if they have not been
active for more than 30 days. This will help keep the issue queue to a manageable size and active for more than 30 days. This will help keep the issue queue to a manageable size and
reduce noise. Should the issue need to stay open, the `keep open` label can be added. reduce noise. Should the issue need to stay open, the `keep open` label can be added.
4. Issue closure 4. Issue closure
@ -120,22 +122,19 @@ contributing to Helm. All issue types follow the same general lifecycle. Differe
2. Fork the desired repo, develop and test your code changes. 2. Fork the desired repo, develop and test your code changes.
3. Submit a pull request. 3. Submit a pull request.
Coding conventions and standards are explained in the official developer docs: Coding conventions and standards are explained in the [official developer docs](https://helm.sh/docs/developers/).
https://github.com/helm/helm/blob/master/docs/developers.md
The next section contains more information on the workflow followed for PRs
## Pull Requests ## Pull Requests
Like any good open source project, we use Pull Requests to track code changes Like any good open source project, we use Pull Requests to track code changes.
### PR Lifecycle ### PR Lifecycle
1. PR creation 1. PR creation
- We more than welcome PRs that are currently in progress. They are a great way to keep track of - We more than welcome PRs that are currently in progress. They are a great way to keep track of
important work that is in-flight, but useful for others to see. If a PR is a work in progress, important work that is in-flight, but useful for others to see. If a PR is a work in progress,
it **must** be prefaced with "WIP: [title]". Once the PR is ready for review, remove "WIP" from it **must** be prefaced with "WIP: [the rest of the title]". Once the PR is ready for review,
the title. remove "WIP" from the title.
- It is preferred, but not required, to have a PR tied to a specific issue. - It is preferred, but not required, to have a PR tied to a specific issue.
2. Triage 2. Triage
- The maintainer in charge of triaging will apply the proper labels for the issue. This should - The maintainer in charge of triaging will apply the proper labels for the issue. This should
@ -148,26 +147,26 @@ Like any good open source project, we use Pull Requests to track code changes
- Reviews from others in the community, especially those who have encountered a bug or have - Reviews from others in the community, especially those who have encountered a bug or have
requested a feature, are highly encouraged, but not required. Maintainer reviews **are** required requested a feature, are highly encouraged, but not required. Maintainer reviews **are** required
before any merge before any merge
- Any PR with the `size/large` label requires 2 review approvals from maintainers before it can be
merged. Those with `size/medium` are per the judgement of the maintainers
4. Reviewing/Discussion 4. Reviewing/Discussion
- Once a maintainer begins reviewing a PR, they will remove the `awaiting review` label and add
the `in progress` label so the person submitting knows that it is being worked on. This is
especially helpful when the review may take awhile.
- All reviews will be completed using Github review tool. - All reviews will be completed using Github review tool.
- A "Comment" review should be used when there are questions about the code that should be - A "Comment" review should be used when there are questions about the code that should be
answered, but that don't involve code changes. This type of review does not count as approval. answered, but that don't involve code changes. This type of review does not count as approval.
- A "Changes Requested" review indicates that changes to the code need to be made before they will be merged. - A "Changes Requested" review indicates that changes to the code need to be made before they will be merged.
- Reviewers should update labels as needed (such as `needs rebase`) - Reviewers should update labels as needed (such as `needs rebase`)
5. Address comments by answering questions or changing code 5. Address comments by answering questions or changing code
6. Merge or close 6. LGTM (Looks good to me)
- Once a Reviewer has completed a review and the code looks ready to merge, an "Approve" review is used
to signal to the contributor and to other maintainers that you have reviewed the code and feel that it is
ready to be merged.
- Any PR against Helm 3 requires 2 review approvals from maintainers before it can be merged, regardless
of PR size. This is to ensure multiple maintainers are aware of any changes going into Helm 3.
7. Merge or close
- PRs should stay open until merged or if they have not been active for more than 30 days. - PRs should stay open until merged or if they have not been active for more than 30 days.
This will help keep the PR queue to a manageable size and reduce noise. Should the PR need This will help keep the PR queue to a manageable size and reduce noise. Should the PR need
to stay open (like in the case of a WIP), the `keep open` label can be added. to stay open (like in the case of a WIP), the `keep open` or `WIP` label can be added.
- If the owner of the PR is listed in `OWNERS`, that user **must** merge their own PRs - If the owner of the PR is listed in the `OWNERS` file, that user **must** merge their own PRs
or explicitly request another OWNER do that for them. or explicitly request another OWNER do that for them.
- If the owner of a PR is _not_ listed in `OWNERS`, any core committer may - If the owner of a PR is _not_ listed in `OWNERS`, any core maintainer may merge the PR.
merge the PR once it is approved.
#### Documentation PRs #### Documentation PRs
@ -190,9 +189,8 @@ The following tables define all label types used for Helm. It is split up by cat
| Label | Description | | Label | Description |
| ----- | ----------- | | ----- | ----------- |
| `bug` | Marks an issue as a bug or a PR as a bugfix | | `bug` | Marks an issue as a bug or a PR as a bugfix |
| `critical` | Marks an issue or PR as critical. This means that addressing the PR or issue is top priority and will be handled first by maintainers | | `critical` | Marks an issue or PR as critical. This means that addressing the PR or issue is top priority and must be addressed as soon as possible |
| `docs` | Indicates the issue or PR is a documentation change | | `docs` | Indicates the issue or PR is a documentation change |
| `duplicate` | Indicates that the issue or PR is a duplicate of another |
| `feature` | Marks the issue as a feature request or a PR as a feature implementation | | `feature` | Marks the issue as a feature request or a PR as a feature implementation |
| `keep open` | Denotes that the issue or PR should be kept open past 30 days of inactivity | | `keep open` | Denotes that the issue or PR should be kept open past 30 days of inactivity |
| `refactor` | Indicates that the issue is a code refactor and is not fixing a bug or adding additional functionality | | `refactor` | Indicates that the issue is a code refactor and is not fixing a bug or adding additional functionality |
@ -201,24 +199,22 @@ The following tables define all label types used for Helm. It is split up by cat
| Label | Description | | Label | Description |
| ----- | ----------- | | ----- | ----------- |
| `help wanted` | This issue is one the core maintainers cannot get to right now and would appreciate help with | | `help wanted` | Marks an issue needs help from the community to solve |
| `proposal` | This issue is a proposal | | `proposal` | Marks an issue as a proposal |
| `question/support` | This issue is a support request or question | | `question/support` | Marks an issue as a support request or question |
| `starter` | This issue is a good for someone new to contributing to Helm | | `good first issue` | Marks an issue as a good starter issue for someone new to Helm |
| `wont fix` | The issue has been discussed and will not be implemented (or accepted in the case of a proposal) | | `wont fix` | Marks an issue as discussed and will not be implemented (or accepted in the case of a proposal) |
### PR Specific ### PR Specific
| Label | Description | | Label | Description |
| ----- | ----------- | | ----- | ----------- |
| `awaiting review` | The PR has been triaged and is ready for someone to review | | `awaiting review` | Indicates a PR has been triaged and is ready for someone to review |
| `breaking` | The PR has breaking changes (such as API changes) | | `breaking` | Indicates a PR has breaking changes (such as API changes) |
| `cncf-cla: no` | The PR submitter has **not** signed the project CLA. |
| `cncf-cla: yes` | The PR submitter has signed the project CLA. This is required to merge. |
| `in progress` | Indicates that a maintainer is looking at the PR, even if no review has been posted yet | | `in progress` | Indicates that a maintainer is looking at the PR, even if no review has been posted yet |
| `needs pick` | Indicates that the PR needs to be picked into a feature branch (generally bugfix branches). Once it has been, the `picked` label should be applied and this one removed | | `needs rebase` | Indicates a PR needs to be rebased before it can be merged |
| `needs rebase` | A helper label used to indicate that the PR needs to be rebased before it can be merged. Used for easy filtering | | `needs pick` | Indicates a PR needs to be cherry-picked into a feature branch (generally bugfix branches). Once it has been, the `picked` label should be applied and this one removed |
| `picked` | This PR has been picked into a feature branch | | `picked` | This PR has been cherry-picked into a feature branch |
#### Size labels #### Size labels
@ -226,11 +222,14 @@ Size labels are used to indicate how "dangerous" a PR is. The guidelines below a
labels, but ultimately this can be changed by the maintainers. For example, even if a PR only makes labels, but ultimately this can be changed by the maintainers. For example, even if a PR only makes
30 lines of changes in 1 file, but it changes key functionality, it will likely be labeled as `size/large` 30 lines of changes in 1 file, but it changes key functionality, it will likely be labeled as `size/large`
because it requires sign off from multiple people. Conversely, a PR that adds a small feature, but requires because it requires sign off from multiple people. Conversely, a PR that adds a small feature, but requires
another 150 lines of tests to cover all cases, could be labeled as `size/small` even though the number another 150 lines of tests to cover all cases, could be labeled as `size/small` even though the number of
lines is greater than defined below. lines is greater than defined below.
| Label | Description | | Label | Description |
| ----- | ----------- | | ----- | ----------- |
| `size/small` | Anything less than or equal to 4 files and 150 lines. Only small amounts of manual testing may be required | | `size/XS` | Denotes a PR that changes 0-9 lines, ignoring generated files. Very little testing may be required depending on the change. |
| `size/medium` | Anything greater than `size/small` and less than or equal to 8 files and 300 lines. Manual validation should be required. | | `size/S` | Denotes a PR that changes 10-29 lines, ignoring generated files. Only small amounts of manual testing may be required. |
| `size/large` | Anything greater than `size/medium`. This should be thoroughly tested before merging and always requires 2 approvals. This also should be applied to anything that is a significant logic change. | | `size/M` | Denotes a PR that changes 30-99 lines, ignoring generated files. Manual validation should be required. |
| `size/L` | Denotes a PR that changes 100-499 lines, ignoring generated files. This should be thoroughly tested before merging and always requires 2 approvals. |
| `size/XL` | Denotes a PR that changes 500-999 lines, ignoring generated files. This should be thoroughly tested before merging and always requires 2 approvals. |
| `size/XXL` | Denotes a PR that changes 1000+ lines, ignoring generated files. This should be thoroughly tested before merging and always requires 2 approvals. |

Write Preview
Loading…
Cancel
Save