Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: update CONTRIBUTING.md merge requirements #3671

Merged
merged 3 commits into from
Mar 15, 2023
Merged

docs: update CONTRIBUTING.md merge requirements #3671

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c065aa8
docs: update CONTRIBUTING.md
dyladan Mar 13, 2023
4898005
Merge branch 'main' into merge-requirements
dyladan Mar 15, 2023
27c14f8
lint: spacing
dyladan Mar 15, 2023
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
84 changes: 50 additions & 34 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,25 @@

We'd love your help!

- [Development Quick Start](#development-quick-start)
- [Pull Request Merge Guidelines](#pull-request-merge-guidelines)
- [General Merge Requirements](#general-merge-requirements)
- [Report a bug or requesting feature](#report-a-bug-or-requesting-feature)
- [How to contribute](#how-to-contribute)
- [Before you start](#before-you-start)
- [Conventional commit](#conventional-commit)
- [Changelog](#changelog)
- [Fork](#fork)
- [Development](#development)
- [Tools used](#tools-used)
- [Install dependencies](#install-dependencies)
- [Compile modules](#compile-modules)
- [Running tests](#running-tests)
- [Linting](#linting)
- [Generating docs](#generating-docs)
- [Adding a package](#adding-a-package)
- [Platform conditional exports](#platform-conditional-exports)

## Development Quick Start

To get the project started quickly, you can follow these steps. For more
Expand All @@ -15,6 +34,37 @@ npm run compile
npm test
```

## Pull Request Merge Guidelines

Most pull requests MAY be merged by an approver OR a maintainer provided they meet the following [General Merge Requirements](#general-merge-requirements).
All requirements are at the discretion of the maintainers.
Maintainers MAY merge pull requests which have not strictly met these requirements.
Maintainers MAY close, block, or put on hold pull requests even if they have strictly met these requirements.

It is generally expected that a maintainer ([@open-telemetry/javascript-maintainers](https://github.com/orgs/open-telemetry/teams/javascript-maintainers)) should review and merge major changes.
Some examples include, but are not limited to:

- API changes
- Breaking changes
- New modules
- Changes which affect runtime support
- New features which are not required by the specification

If a PR has not been interacted with by a reviewer within one week, please ping the approvers ([@open-telemetry/javascript-approvers](https://github.com/orgs/open-telemetry/teams/javascript-approvers)).

### General Merge Requirements

- No “changes requested” reviews by approvers, maintainers, technical committee members, or subject matter experts
- No unresolved conversations
- Approved by at least one maintainer OR by at least one approver who is not the approver merging the pull request
- A pull request for small (simple typo, URL, update docs, or grammatical fix) changes may be approved and merged by the same approver
- For plugins, exporters, and propagators approval of the original code module author, or a contributor who has done extensive work on the module, is preferred but not required
- New or changed functionality is tested by unit tests
- New or changed functionality is documented if appropriate
- Substantial changes should not be merged within 24 hours of opening in order to allow reviewers from all time zones to have a chance to review

If all of the above requirements are met and there are no unresolved discussions, a pull request may be merged by either a maintainer or an approver.

## Report a bug or requesting feature

Reporting bugs is an important contribution. Please make sure to include:
Expand Down Expand Up @@ -216,40 +266,6 @@ To add a new package, copy `packages/template` to your new package directory and

After adding the package, run `npm install` from the root of the project. This will update the `tsconfig.json` project references automatically and install all dependencies in your new package. For packages supporting browser, file `tsconfig.esm.json` needs to be manually updated to include reference to ES modules build.

### Guidelines for Pull Requests

- Typically we try to turn around reviews within one to two business days.
- It is generally expected that a maintainer ([@open-telemetry/javascript-maintainers](https://github.com/orgs/open-telemetry/teams/javascript-maintainers)) should review and merge every PR.
- If a change has met the requirements listed below, an approver may also merge the pull request.
- Most PRs should be merged in one to two weeks.
- If a PR is taking longer than 30 days, please ping the approvers ([@open-telemetry/javascript-approvers](https://github.com/orgs/open-telemetry/teams/javascript-approvers)) as it may have been lost
- Dependency upgrades and Security fixes: This PR is small and/or low-risk and can be merged with only maintainer reviews.
- If your patch is not getting reviewed or you need a specific person to review it, you can @username or @open-telemetry/javascript-approvers a reviewer asking for a review in the pull request.
- API changes, breaking changes, or large changes will be subject to more scrutiny and may require more reviewers. These PRs should only be merged by maintainers.
- Changes to existing plugins and exporters will typically require the approval of the original plugin/exporter author.

### General Merge Requirements

- All requirements are at the discretion of the maintainers.
- Maintainers may merge pull requests which have not strictly met these requirements.
- Maintainers may close, block, or put on hold pull requests even if they have strictly met these requirements.
- No “changes requested” reviews.
- No unresolved conversations.
- 3 approvals, including the approvals of at least 2 maintainers
- A pull request opened by an approver may be merged with only the 2 maintainer reviews.
- Small (simple typo, URL, update docs, or grammatical fix) or high-priority changes may be merged more quickly or with fewer reviewers at the discretion of the maintainers. This is typically indicated with the express label.
- For plugins, exporters, and propagators approval of the original code module author is preferred but not required.
- New or changed functionality is tested by unit tests.
- New or changed functionality is documented.
- Substantial changes should not be merged within 24 hours of opening in order to allow reviewers from all time zones to have a chance to review.

If all of the above requirements are met and there are no unresolved discussions, a pull request may be merged by either a maintainer or an approver.

### Generating CHANGELOG documentation

- Generate and export your [Github access token](https://docs.github.com/en/github/authenticating-to-github/creating-a-personal-access-token): `export GITHUB_AUTH=<your_token>`
- `npm run changelog` to generate CHANGELOG documentation in your terminal (see [RELEASING.md](RELEASING.md) for more details).

### Platform conditional exports

Universal packages are packages that can be used in both web browsers and
Expand Down