Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Add pull request process to CONTRIBUTING docs #892

Merged
merged 3 commits into from
Oct 20, 2021
Merged

Add pull request process to CONTRIBUTING docs #892

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a911c6
Add pull request process to CONTRIBUTING docs
mcwumbly Oct 14, 2021
84bfb97
Remove reference to `needs-triage` label for PRs
mcwumbly Oct 20, 2021
5652c66
Remove outdated issues lifecycle section
mcwumbly Oct 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 86 additions & 24 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,32 +27,18 @@ not have a slack channel or email list (hopefully soon).

## Propose a Change

Pull requests are welcome for all changes. When adding new functionality, we
encourage including test coverage. If significant effort will be involved, we
suggest beginning by submitting an issue so any high level feedback can be
addressed early.
We suggest beginning by submitting an issue so any high level feedback can be
addressed early, particularly if significant effort will be involved.

Please submit feature requests and bug reports by using GitHub issues and filling
the supplied template with as much detail as you can.
in the supplied template with as much detail as you can.

Before submitting an issue, please search through open ones to ensure others
have not submitted something similar. If a similar issue exists, please add any
additional information as a comment.

### Pull Request Etiquette

* Before submitting a pull request, please make sure you verify the changes
locally. The `Makefile` in this repository provides useful targets such as
`lint` and `test` to make verification easier.
* Prefer small commits and small pull requests over large changes.
Small changes are easier to digest and get reviewed faster. If you find
that your change is getting large, break up your PR into small, logical
commits. Consider breaking up large PRs into smaller PRs, if each of them
can be useful on its own.
* Pull requests *must* include a `Fixes #NNNN` or `Updates #NNNN` comment. Remember
that `Fixes` will close the associated issue, and `Updates` will link the PR to it.
* Have good commit messages. Please see [Commit Messages](#commit-messages)
section for guidelines.
<!-- TODO: this section should be revised, but it needs discussion first.
https://github.com/vmware-tanzu/tanzu-framework/issues/874

### Issues Lifecycle

Expand All @@ -61,6 +47,24 @@ investigating it. We keep `in-progress` issues open until they have been
resolved and released. Once released, a comment containing release information
will be posted in the issue's thread.

-->

## Contribute a Change

Pull requests are welcome for all changes, whether they are improving
documentation, fixing a bug, adding or enhancing a feature, or fixing a typo.

Changes to the behavior of the `tanzu-framework` itself will require that you
build and test your changes.

When adding new functionality, or fixing bugs, add appropriate test coverage
where possible. Different parts of the code base have different strategies and
patterns for testing, some of which may be in flux at any point in time.
Consider commenting on the issue to seek input or or opening a draft pull
request to seek feedback on approaches to testing a particular change.

To build the project from source, please consider the docs on [local development](docs/dev/build.md).

### Commit Messages

* Commit messages should include a short (72 chars or less) title summarizing the change.
Expand All @@ -70,7 +74,69 @@ will be posted in the issue's thread.
* Bulleted points are fine.
* Typically a hyphen or asterisk is used for the bullet, followed by a single space.

### Merging Commits
## Pull Request Process

### Creating a Pull Request

Use the pull request template to provide a complete description of the change.
The template aims to capture important information to streamline the review
process, ensure your changes are captured in release notes, and update related
issues. Your pull request description and any discussion that follows is a
contribution itself that will help the community and future contributors
understand the project better.

* Before submitting a pull request, please make sure you verify the changes
locally. The `Makefile` in this repository provides useful targets such as
`lint` and `test` to make verification easier.
* Prefer small commits and small pull requests over large changes.
Small changes are easier to digest and get reviewed faster. If you find
that your change is getting large, break up your PR into small, logical
commits. Consider breaking up large PRs into smaller PRs, if each of them
can be useful on its own.
* Have good commit messages. Please see [Commit Messages](#commit-messages)
section for guidelines.
* Pull requests *should* reference an existing issue and include a `Fixes #NNNN`
or `Updates #NNNN` comment. Remember that `Fixes` will close the associated
issue, and `Updates` will link the PR to it.

### Getting your Pull Request Reviewed, Approved, and Merged

Once a pull request has been opened, the following must take place before it is merged:

* It needs the `ok-to-merge` label
* It needs to pass all the checks.
* It needs to be approved by a [CODEOWNER](https://github.com/vmware-tanzu/tanzu-framework/blob/main/CODEOWNERS) for all files changed

While these steps will not always take place in the same order, the following describes the process for a typical pull request once it is opened:

1. Review is automatically requested from CODEOWNERS and `needs-triage` label is automatically added.
mcwumbly marked this conversation as resolved.
Show resolved Hide resolved
2. Assignee is added to pull request to ensure it gets proper attention throughout the process.
Typically one of the CODEOWNERS will assign themselves, but they may choose to delegate to someone else.
3. Triage removes `needs-triage` and adds `ok-to-merge` if the pull request is generally aligned with product goals and does not conflict with current milestones; otherwise they may add a comment and a `do-not-merge/*` label.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to read this line a couple of times to make sure I knew who Triage was. I assume, based on discussion in SIG meetings and context clues, that Triage is referring to a group of tanzu-framework product owners. Do you think there is any chance we need to be more specific about who Triage is so that community members know exactly who is on the hook for this step?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ankeesler I think it could be some combination of: 1) the assignee, 2) tanzu-framework-reviewers, or 3) a person wearing the "triage" hat during some periodic triage meeting... without more context on how that is evolving, I was weary of being overly specific here.

In the meantime, my hope is that this final paragraph offers enough clarity on what to do if things are not progressing:

If any of the above is unclear, and there has been no new activity for 2-3 days,
the contributor is encouraged to seek further information by commenting and
mentioning the assignee or @vmware-tanzu/tanzu-framework-reviewers if there is
no assignee or they themselves are unresponsive.

cc @vuil

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we can iterate and provider more clarifications when the triage process is more nailed down. For now, I think the quoted paragraph is a useful catch-all on what actions to take when things are not progressing.

4. Assignee may request others to do an initial review; anyone else may review
5. Reviewers leave feedback
6. Contributor updates pull request to address feedback
7. Requested reviewer approves pull request
8. Assignee approves pull request
9. Assignee merges pull request or requests another member to merge it if
necessary.

During the review process itself, direct discussion among contributors and reviewers is encouraged.

Throughout the process, and until the pull request has been merged, the following should be transparent to the contributor:

* Has the pull request been assigned to anyone yet?
* Has the pull request been triaged yet?
* Has someone been requested to review the pull request?
* Has the PR been approved by a reviewer?
* Has the PR been approved by the approver?

If any of the above is unclear, and there has been no new activity for 2-3 days,
the contributor is encouraged to seek further information by commenting and
mentioning the assignee or @vmware-tanzu/tanzu-framework-reviewers if there is
no assignee or they themselves are unresponsive.

### Merging a Pull Request

Maintainers should prefer to merge pull requests with the [Squash and merge](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#squash-and-merge-your-pull-request-commits) option.
This option is preferred for a number of reasons.
Expand All @@ -87,10 +153,6 @@ cleanly separated from semantic changes. The maintainer should review commit
messages for each commit and make sure that each commit builds and passes
tests.

### Building From Source

To build the project from source, please consider the docs on [local development](docs/dev/build.md).

## Contributor License Agreement

All contributors to this project must have a signed Contributor License
Expand Down