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

Add practices to contributor guidelines #4702

Merged
merged 6 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
72 changes: 0 additions & 72 deletions content/en/docs/contributing/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ title: Development
description:
Learn how to set up a development environment for the opentelemetry.io site.
weight: 60
cSpell:ignore: chalin
---

The following instructions explain how to set up a development environment for
Expand Down Expand Up @@ -122,77 +121,6 @@ within a submodule, set the environment variable `GET=no`. You also need to run
`git fetch --unshallow` the submodule before you can submit a PR. Alternatively,
set `DEPTH=100` and re-fetch submodules.

## Approver and maintainer practices

This last section includes guidelines and some common practices used by
approvers and maintainers while doing code reviews:

- PRs with changes to documentation co-owned by a SIG (collector, demo,
language-specific...) should aim for two approvals: one by a docs approver and
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
- 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.
- A PR by non-maintainers should **never** update git sub modules. This happens
by accident from time to time. Let the PR author know that they should not
worry about it, we will fix this before merging, but in the future they should
make sure that they work from an up-to-date fork.
- If the contributor is having trouble signing the CLA or used the wrong email
by mistake in one of their commits, ask them to fix the issue or rebase the
pull request. Worst case scenario, close and re-open the PR to trigger a new
CLA check.
- 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, rebase 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=<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
[fork]: https://docs.github.com/en/get-started/quickstart/fork-a-repo
Expand Down
132 changes: 132 additions & 0 deletions content/en/docs/contributing/sig-practices.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
---
title: SIG practices for approver and maintainers
linkTitle: SIG practices
description: Learn how approvers and maintainers manage issues and contributions
svrnm marked this conversation as resolved.
Show resolved Hide resolved
weight: 999
cSpell:ignore: chalin Comms docsy onboarded
---

This pages includes guidelines and some common practices used by approvers and
maintainers.

## Onboarding

If a contributor steps up to take on a role with more responsibility towards the
documentation (approver, maintainer) they will be onboarded by existing
approvers and maintainers:

- They are added to the `docs-approvers` (or `docs-maintainers`) group
- They are added to the `#otel-comms` and `#otel-maintainers` and private
in-team slack channels
- They are asked to enroll for the calendar invites for
[SIG Comms meeting](https://groups.google.com/a/opentelemetry.io/g/calendar-comms)
and
[maintainers meeting](https://groups.google.com/a/opentelemetry.io/g/calendar-maintainer-meeting)
- They are asked to verify that the current meeting time for SIG Comms works for
them and if not to collaborate with existing approvers and maintainers to find
a time that suits everyone.
- They are asked to review the different resources available for contributors:
- [Community Resources](https://github.com/open-telemetry/community/),
especially the document around
[Community Membership](https://github.com/open-telemetry/community/blob/main/community-membership.md)
and the
[social media guide](https://github.com/open-telemetry/community/blob/main/social-media-guide.md).
- [Contributing Guidelines](/docs/contributing) As part of this, they will
review those documents and provide feedback for improving them via issues or
pull requests.
svrnm marked this conversation as resolved.
Show resolved Hide resolved

Additional valuable resources to review are

- [Hugo Documentation](https://gohugo.io/documentation/)
- [docsy Documentation](https://www.docsy.dev/docs/)
- [Marketing Guidelines](/community/marketing-guidelines/), including the Linux
Foundation’s branding and
[trademark usage guidelines](https://www.linuxfoundation.org/legal/trademark-usage).
Those are especially valuable when reviewing entries to the registry,
integrations, vendors, adopters or distributions.
svrnm marked this conversation as resolved.
Show resolved Hide resolved

## Collaboration

- 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 the [Community Values](/community/mission/#community-values). They 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.
svrnm marked this conversation as resolved.
Show resolved Hide resolved

## Code Reviews
svrnm marked this conversation as resolved.
Show resolved Hide resolved

### General

- 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.
- A PR by non-maintainers should **never** update git sub modules. This happens
by accident from time to time. Let the PR author know that they should not
worry about it, we will fix this before merging, but in the future they should
make sure that they work from an up-to-date fork.
- If the contributor is having trouble signing the CLA or used the wrong email
by mistake in one of their commits, ask them to fix the issue or rebase the
pull request. Worst case scenario, close and re-open the PR to trigger a new
CLA check.
- 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.

### Co-owned PRs

PRs with changes to documentation co-owned by a SIG (collector, demo,
language-specific...) should aim for two approvals: one by a docs approver and
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
svrnm marked this conversation as resolved.
Show resolved Hide resolved

### PRs from bots

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.
svrnm marked this conversation as resolved.
Show resolved Hide resolved

### Translation PRs

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.

### Merging PRs

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, rebase 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:
svrnm marked this conversation as resolved.
Show resolved Hide resolved

```shell
export PR=<ID OF THE PR>; gh pr checks ${PR} --watch && gh pr merge ${PR} --squash
```
16 changes: 16 additions & 0 deletions static/refcache.json
Original file line number Diff line number Diff line change
Expand Up @@ -4891,6 +4891,10 @@
"StatusCode": 206,
"LastSeen": "2024-06-06T14:51:47.628909-04:00"
},
"https://gohugo.io/documentation/": {
"StatusCode": 206,
"LastSeen": "2024-06-18T10:36:41.646389+02:00"
},
"https://gohugo.io/methods/site/regularpages/": {
"StatusCode": 206,
"LastSeen": "2024-06-07T12:38:33.735717-04:00"
Expand Down Expand Up @@ -4971,6 +4975,14 @@
"StatusCode": 206,
"LastSeen": "2024-01-18T19:02:58.077563-05:00"
},
"https://groups.google.com/a/opentelemetry.io/g/calendar-comms": {
"StatusCode": 200,
"LastSeen": "2024-06-18T10:36:39.590318+02:00"
},
"https://groups.google.com/a/opentelemetry.io/g/calendar-maintainer-meeting": {
"StatusCode": 200,
"LastSeen": "2024-06-18T10:36:41.220362+02:00"
},
"https://grpc.github.io/grpc/core/md_doc_naming.html": {
"StatusCode": 206,
"LastSeen": "2024-01-18T08:53:24.217022-05:00"
Expand Down Expand Up @@ -8779,6 +8791,10 @@
"StatusCode": 200,
"LastSeen": "2024-01-30T06:08:30.61479-05:00"
},
"https://www.docsy.dev/docs/": {
"StatusCode": 206,
"LastSeen": "2024-06-18T10:36:42.507515+02:00"
},
"https://www.dynatrace.com/news/blog/what-is-opentelemetry-2/": {
"StatusCode": 200,
"LastSeen": "2024-01-18T19:10:40.326174-05:00"
Expand Down