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.

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

Update approver and maintainer practices #4684

Merged
merged 8 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
38 changes: 38 additions & 0 deletions content/en/docs/contributing/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,27 @@ approvers and maintainers while doing code reviews:
one by a SIG approver:
- Doc approver label such PRs with `sig:<name>` and tag the SIG `-approvers`
group on that PR
- After a doc approver has reviewed and approved the PR, they can add the
label
[`sig-approval-missing`](https://github.com/open-telemetry/opentelemetry.io/labels/sig-approval-missing).
This signals to the SIG that they need to handle the PR
- If no SIG approval is given within a certain grace period (two weeks in
general, but may be less in urgent cases), docs maintainer may use their own
judgement to merge that PR
- PRs created by bots can be merged by the following practice:
- PRs that auto-update versions in the registry can be fixed, approved and
merged immediately
- PRs that auto-update the versions of SDKs, zero-code instrumentations or the
collector can be approved and merged except the corresponding SIG signals
that merging should be postponed
svrnm marked this conversation as resolved.
Show resolved Hide resolved
- PRs that auto-update the version of any specification often require updates
to scripts for the CI checks to pass. In that case often
[@chalin](https://github.com/chalin/) will handle that PR. Otherwise those
PRs can as well be approved and merged except the corresponding SIG signals
that merging should be postponed
svrnm marked this conversation as resolved.
Show resolved Hide resolved
- PRs with changes to translations should aim for two approvals: one by a docs
approver and one by a translation approver. Similar practices apply as
suggested for the co-owned PRs.
- If the PR branch is `out-of-date with the base branch`, they do not need to be
updated continuously: every update triggers all the PR CI checks to be run!
It's often enough to update them before merging.
Expand All @@ -148,11 +166,31 @@ approvers and maintainers while doing code reviews:
- Words unknown to cspell should be added to the cspell ignore list per page by
PR authors. Only approvers and maintainers will add commonly used terms to the
global list.
- Approvers and maintainers have different work schedules and circumstances.
That's why all communication is assumed to be asynchronously and they should
not feel obligated to reply outside of their normal schedule.
- When an approver or maintainer won't be available to contribute for an
extended period of time (more than a few days or a week) or won't be available
in that period of time, they should communicate this using the
[#otel-comms](https://cloud-native.slack.com/archives/C02UN96HZH6) channel and
updating the GitHub status.
- Approver and maintainer adhere to the
[OTel Code of Conduct](https://github.com/open-telemetry/community/?tab=coc-ov-file#opentelemetry-community-code-of-conduct)
and are friendly and helpful towards contributors. In the case of a conflict,
misunderstanding or any other kind of situation that makes an
approver/maintainer feel uncomfortable they can step back from a conversation,
issue or PR and ask another approver/maintainer to step in.

The following workflow can be applied by maintainers to merge PRs:

- Make sure that a PR has all approvals and all CI checks pass
- If the branch is out-of-date, update it via the GitHub UI.
svrnm marked this conversation as resolved.
Show resolved Hide resolved
- The update will trigger all CI checks to run again, wait for them to pass or
execute a script like the following to make it happen in the background:

```shell
export PR=<ID OF THE PR>; gh pr checks ${PR} --watch && gh pr merge ${PR} --squash
```

[clone]:
https://docs.github.com/en/repositories/creating-and-managing-repositories/cloning-a-repository
Expand Down
8 changes: 8 additions & 0 deletions static/refcache.json
Original file line number Diff line number Diff line change
Expand Up @@ -2503,6 +2503,10 @@
"StatusCode": 200,
"LastSeen": "2024-01-30T15:25:16.58574-05:00"
},
"https://github.com/chalin/": {
"StatusCode": 200,
"LastSeen": "2024-06-14T09:35:29.965101669Z"
},
"https://github.com/cloudflare/cfssl": {
"StatusCode": 200,
"LastSeen": "2024-01-18T19:36:55.954418-05:00"
Expand Down Expand Up @@ -4311,6 +4315,10 @@
"StatusCode": 200,
"LastSeen": "2024-01-18T19:09:43.270324-05:00"
},
"https://github.com/open-telemetry/opentelemetry.io/labels/sig-approval-missing": {
"StatusCode": 200,
"LastSeen": "2024-06-14T09:35:27.61260596Z"
},
"https://github.com/open-telemetry/opentelemetry.io/pull/2130": {
"StatusCode": 200,
"LastSeen": "2024-01-30T15:26:13.725828-05:00"
Expand Down
Loading