8000 docs: formalized '1 approval' and 'team assigned' labels (#9861) · abrahamguo/typescript-eslint@875e03d · GitHub
[go: up one dir, main page]
Skip to content

Commit 875e03d

Browse files
JoshuaKGoldbergkirkwaiblinger
JoshuaKGoldberg
and
kirkwaiblinger
authored
docs: formalized '1 approval' and 'team assigned' labels (typescript-eslint#9861)
* docs: formalized '1 approval' and 'team assigned' labels * Update docs/maintenance/Pull_Requests.mdx Co-authored-by: Kirk Waiblinger <kirk.waiblinger@gmail.com> * Mention time critical tasks * Update docs/maintenance/Pull_Requests.mdx * remove unnecessary tip * Added note on waiting periods --------- Co-authored-by: Kirk Waiblinger <kirk.waiblinger@gmail.com>
1 parent 8b1eb00 commit 875e03d

File tree

3 files changed

+23
-7
lines changed

3 files changed

+23
-7
lines changed

‎.github/workflows/pr-review-requested.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ jobs:
44
steps:
55
- uses: actions-ecosystem/action-remove-labels@v1
66
with:
7-
labels: 'awaiting response'
7+
labels: |
8+
1 approval
9+
awaiting response
810
- if: failure()
911
run: |
1012
echo "Don't worry if the previous step failed."

‎docs/maintenance/Issues.mdx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,9 @@ Most issues go through the following review flow when created or updated:
6060
- Close the issue as _not planned_
6161
- If the issue requires further discussion or community engagement evaluation:
6262
- Add the `evaluating community engagement` label and remove the `triage` label
63-
2. If the report is valid, add the `accepting prs` label and remove the `triage` label
63+
2. If the report is valid:
64+
- Remove the `triage` label
65+
- Add the `team assigned` label if you think only a member of the team should tackle it, or `accepting prs` if anybody could
6466
3. If you know the rough steps for a fix, consider writing a comment with links to codebase to help someone put together a fix
6567
4. If you think that the fix is relatively straightforward, consider also adding the `good first issue` label
6668

@@ -97,7 +99,7 @@ However, there are cases when maintainers can't confidently choose the most reas
9799

98100
3-6 months after the issue is labeled `evaluating community engagement`, the engagement of community is evaluated:
99101

100-
- If the community was active and common ground was found, the issue is labeled as `accepting prs`
102+
- If the community was active and common ground was found, the issue is labeled as `accepting prs` or `team assigned`
101103
- Otherwise, the issue is closed as _not planned_ with the `wontfix` label
102104

103105
For requests that can be implemented outside of typescript-eslint, be sure to mention any relevant APIs such as [Custom Rules](../developers/Custom_Rules.mdx) that can be used.

‎docs/maintenance/Pull_Requests.mdx

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ Open pull requests ideally switch between two states:
3737
- Ready for review: either newly created or after [review is re-requested](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/about-pull-request-reviews#re-requesting-a-review)
3838
- Waiting for author activity: either by virtue of [being a draft](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests#draft-pull-requests) or having the [`awaiting response` label](https://github.com/typescript-eslint/typescript-eslint/pulls?q=is%3Apr+is%3Aopen+label%3A%22awaiting+response%22)
3939

40-
Add the `awaiting response` label to a PR whenever you post a review.
40+
Add the `awaiting response` label to a PR and remove `1 approval` if it exists whenever you post a request for changes.
4141
It will be automatically removed if the author re-requests review.
4242

4343
### Be Kind
@@ -66,9 +66,6 @@ If there's no backing issue:
6666
- If the PR is a straightforward docs or tooling fix that doesn't impact users, it's ok to review it as-is
6767
- Otherwise, add the `please fill out the template` label and post a comment like the _Needs Issue_ template
6868

69-
Upon completing your review, if the build is passing and you feel comfortable merging it in, go ahead and squash merge.
70-
Otherwise, add the `1 approval` label.
71-
7269
#### Common Things to Look For
7370

7471
- Style: that can you read through the code easily, there are comments for necessary tricky areas, and not unnecessary comments otherwise.
@@ -111,6 +108,21 @@ For preliminary reviews, be clear with what scale you're reviewing at: _"Reviewi
111108
For repeat reviews, be clear when it's something you missed earlier and/or there's new information.
112109
Don't apologize if the missed information was only made clear because of requested changes - this is part of the review process!
113110

111+
### Approvals
112+
113+
If the PR addresses a time critical task, such as a security fix or `main` branch build break, go ahead and squash merge it.
114+
115+
Otherwise, upon completing your review, if the build is passing and you have no blockers, approve it on GitHub.
116+
Then:
117+
118+
- If there isn't a `1 approval` label or existing approval, add the `1 approval` label
119+
- If there's already `1 approval` and/or it's been a week since the last request for changes, go ahead and squash merge
120+
- For straightforward PRs that don't impact users, you can wait 3 days instead
121+
122+
There's no need to reset waiting periods for minor fixups from code reviews of otherwise approved code.
123+
124+
We try to leave PRs open for at least a week to give reviewers who aren't active every day a chance to get to it.
125+
114126
### Other States
115127

116128
#### External Blockers

0 commit comments

Comments
 (0)
0