From e356e2ce17e4d3e8b0cf5c4bf84141cb9b345307 Mon Sep 17 00:00:00 2001 From: Severin Neumann Date: Fri, 14 Jun 2024 11:33:20 +0200 Subject: [PATCH] Update approver and maintainer practices --- content/en/docs/contributing/development.md | 27 +++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/content/en/docs/contributing/development.md b/content/en/docs/contributing/development.md index 28b05d316f38..f469e907e06d 100644 --- a/content/en/docs/contributing/development.md +++ b/content/en/docs/contributing/development.md @@ -131,9 +131,22 @@ approvers and maintainers while doing code reviews: one by a SIG approver: - Doc approver label such PRs with `sig:` 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 + - 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 +- 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. @@ -148,12 +161,26 @@ 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. +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. +- 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=; gh pr checks ${PR} --watch && gh pr merge ${PR} --squash + ``` + [clone]: https://docs.github.com/en/repositories/creating-and-managing-repositories/cloning-a-repository [fork]: https://docs.github.com/en/get-started/quickstart/fork-a-repo