-
Notifications
You must be signed in to change notification settings - Fork 932
Clarify how to make progress on a PR #745
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.
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
Changes from 11 commits
3600e61
e94cbbb
4bee18e
be91945
204657e
41ccd74
de294b2
5452b97
1e88ea3
b2fd829
b27a220
d58806a
de31de8
a10ab8b
06d1d7a
c6d99a6
9011c8c
6dba387
52035db
4c08bed
e77f809
e073a65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -82,3 +82,70 @@ To quickly fix typos, use | |||||
| ```bash | ||||||
| make misspell-correction | ||||||
| ``` | ||||||
|
|
||||||
| ## Pull Request | ||||||
|
|
||||||
| ### How to send PR | ||||||
reyang marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
|
|
||||||
| Everyone is welcome to contribute to `OpenTelemetry Specification` via GitHub | ||||||
reyang marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| pull requests (PRs). | ||||||
|
|
||||||
| To create a new PR, fork the project in GitHub and clone the upstream repo: | ||||||
reyang marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
|
|
||||||
| ```sh | ||||||
| git clone https://github.com/open-telemetry/opentelemetry-specification.git | ||||||
| ``` | ||||||
|
|
||||||
| Add your fork as a remote: | ||||||
|
|
||||||
| ```sh | ||||||
| git remote add fork https://github.com/YOUR_GITHUB_USERNAME/opentelemetry-specification.git | ||||||
| ``` | ||||||
|
|
||||||
| Check out a new branch, make modifications and push the branch to your fork: | ||||||
|
|
||||||
| ```sh | ||||||
| $ git checkout -b feature | ||||||
| # edit files | ||||||
| $ git commit | ||||||
| $ git push fork feature | ||||||
| ``` | ||||||
|
|
||||||
| Open a pull request against the main `opentelemetry-specification` repo. | ||||||
|
|
||||||
| If the PR is not ready for review, please mark it as | ||||||
| [`draft`](https://github.blog/2019-02-14-introducing-draft-pull-requests/). | ||||||
|
|
||||||
| Please make sure CLA is signed and CI is clear. We don't expect people to review | ||||||
| and comment on a PR that doesn't have CLA signed. | ||||||
|
|
||||||
| ### How to get PR merged | ||||||
reyang marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
|
|
||||||
| A PR is considered to be **ready to merge** when: | ||||||
|
|
||||||
| * It has received two approvals from the [code owners](./.github/CODEOWNERS) (at | ||||||
reyang marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| different companies). | ||||||
| * There is no `request changes` from the [code owners](./.github/CODEOWNERS). | ||||||
carlosalberto marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| * It has been at least two working days since the last modification (except for | ||||||
carlosalberto marked this conversation as resolved.
Show resolved
Hide resolved
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd also add that any significant change should be clearly mentioned in a separate comment to not go unnoticed.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Same as the previous comment) |
||||||
| the trivial updates, such like typo, cosmetic, rebase, etc.). This gives | ||||||
| people reasonable time to review. | ||||||
| * Trivial changes (typos, cosmetic changes, CI improvements, etc.) don't have to wait for | ||||||
| two days. | ||||||
|
|
||||||
| Any [spec | ||||||
| approver](https://github.com/orgs/open-telemetry/teams/specs-approvers) can | ||||||
| merge the PR once it is **ready to merge**. | ||||||
reyang marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
|
||||||
| If a PR has been stuck (e.g. there are lots of debates and people couldn't agree | ||||||
| on each other), the owner should try to get people aligned by: | ||||||
reyang marked this conversation as resolved.
Show resolved
Hide resolved
carlosalberto marked this conversation as resolved.
Show resolved
Hide resolved
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| * Consolidating the perspectives and putting a summary in the PR. | ||||||
reyang marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
carlosalberto marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| * Tagging subdomain experts (by looking at the change history) in the PR asking | ||||||
| for suggestion. | ||||||
| * Reaching out to more people on the [gitter | ||||||
| channel](https://gitter.im/open-telemetry/opentelemetry-specification). | ||||||
| * Stepping back to see if it makes sense to narrow down the scope of the PR or split it up. | ||||||
|
|
||||||
| If none of the above worked and the PR has been stuck for more than 2 weeks, the | ||||||
| owner should bring it to the [OpenTelemetry Specification SIG | ||||||
| meeting](https://github.com/open-telemetry/community#cross-language-specification). | ||||||
Uh oh!
There was an error while loading. Please reload this page.