Skip to content
This repository was archived by the owner on Jul 1, 2024. It is now read-only.

Commit dcb5a84

Browse files
committed
Revise staleness timelines and timeline handling
* `staleness` is now an optional object with fields for the `state` (fresh / attention / nearly / done) `kind` (which is also the label that will be applied when the timeline is done), and `days` for the count (replacing `stalenessInDays`). It's `undefined` when we're not on any of the timelines. * Shuffle the code so staleness handling is used in the same place regardless of other state. Note that it's done regardless of other actions which leads to some changes test in tests, but in practice it shouldn't be a problem (unless the bot dies for many days). * The `Staleness` generation becomes the place for configuring staleness timelines, together with mappings in `comments.ts` for the explanation and comments of various states. * Added the `Unreviewed` kind --- changed the 44857 test to use it instead of `Abandoned`. * Staleness also has a `doTimelineActions` which is now the central place for doing all timeline-related actions. * Change: the stalness comment tags are computed (in `makeStaleness`), they depend on the state: - `nearly`: a tag that includes the date that the countdown started at, so these comments can be repeated if there's a timeline change that voided the previous staleness, but a new one triggered. - `done`: no date in the tag, since these comments are terminal. * Change: `YSYL` is renamed to `Unmerged`. * Change: put the `Unmerged`/`Abandoned`/`Unreviewd` label when reaching `nearly` state, so it is useful to find PRs that are approaching their terminal (`done`) state. * Revise the timelines, with some minor changes, and update all comments accordingly. Notable changes from #208: - The waits are slightly different, but roughly the same. - "Becoming mergable" is measured from the merge offer comment when found. - The unreviewed timeline counts days from the last update rather than from the creation time. This because I see a lot of PR's that have a few days of work following their creation. It could be confusing if the count starts at creation, since some of these PRs would seem to be announced unreviewed very quickly. * Update `docs/how-it-works.md`. Closes #208 Closes #207 Closes #204
1 parent df1a976 commit dcb5a84

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+883
-192
lines changed

docs/how-it-works.md

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
_Disclaimer: This could be out of date, the source of truth is always [compute-pr-actions.ts](https://github.com/DefinitelyTyped/dt-mergebot/blob/master/src/compute-pr-actions.ts)_
1+
_Disclaimer: This could be out of date, the source of truth is always [compute-pr-actions]_
22

33
<img src="./dt-mergebot-lifecycle.png" />
44
<!-- https://www.figma.com/file/qE7BDiEucqI55Q9u0yZO3b/dt-pr-lifecycle?node-id=6%3A7 -->
@@ -21,26 +21,34 @@ _Disclaimer: This could be out of date, the source of truth is always [compute-p
2121
PRs that are blessed (see `info.maintainerBlessed`) are excluded when
2222
possible.
2323

24-
### Idle PR Removal
24+
### Stale PRs
2525

26-
When a PR:
26+
There are several categories for PRs getting stale: "Unmerged" (good to go, but
27+
author+owners didn't request to merge them), "Abandoned" (bad CI, change
28+
requests), "Unreviewed" (got no reviews). Each of these starts a day countdown
29+
at a configurable point, and goes through several states at configurable day
30+
counts:
2731

28-
- Has merge conflicts, CI is failing or has Reviews which reject the most recent commit
29-
- Has not had any commits/comments/reviews/review comments in the last three weeks
32+
- `fresh`: just entered the corresponding staleness timeline, nothing done.
33+
- `attention`: the inactivity is now shown on the welcome message with a brief
34+
explanation.
35+
- `nearly`: a comment is posted and a label with the staleness category is
36+
added.
37+
- `done`: the timeline is done, either move the PR to a column (unreviewed) or
38+
close it (the other).
3039

31-
it will get a ping that it has about a week for something to happen.
40+
See the `getStaleness` definition in [compute-pr-actions] for the current
41+
configuration (conditions, count start, day counts, and final action).
3242

33-
Then, assuming no new activities, a PR which:
43+
The explanations for `attention` and the posted comments are defined in
44+
`StalenessExplanations` and `StalenessComment` respectively (in [comments]).
3445

35-
- Has merge conflicts, CI is failing or has Reviews which reject the most recent commit
36-
- Has not had any commits/comments/reviews/review comments in the last 30 days
37-
38-
will be closed.
46+
### Cleanup
3947

40-
For PRs that are ready to merge but were not, there is a similar (but
41-
much shorter) progression: pinged after 4 days, and moved to YSYL state
42-
after 8.
48+
A [daily script](../src/scripts/daily.ts) is running every night, cutting the
49+
`Recently Merged` column to 50. It also removes closed PRs from other columns
50+
as a safeguard in case the bot missed a PR closing event.
4351

44-
### Cleanup
4552

46-
A [daily script](../src/scripts/daily.ts) is running every night, cutting the `Recently Merged` column to 50. It also removes closed PRs from other columns as a safeguard in case the bot missed a PR closing event.
53+
[compute-pr-actions]: <https://github.com/DefinitelyTyped/dt-mergebot/blob/master/src/compute-pr-actions.ts>
54+
[comments]: <https://github.com/DefinitelyTyped/dt-mergebot/blob/master/src/comments.ts>

src/_tests/cachedQueries.json

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,11 @@
163163
"name": "Mergebot Error",
164164
"__typename": "Label"
165165
},
166+
{
167+
"id": "MDU6TGFiZWwyNTI5Mjc2Njc2",
168+
"name": "Multiple Languages",
169+
"__typename": "Label"
170+
},
166171
{
167172
"id": "MDU6TGFiZWw2NDY3ODg4ODg=",
168173
"name": "New Definition",
@@ -253,6 +258,11 @@
253258
"name": "Unmergeable",
254259
"__typename": "Label"
255260
},
261+
{
262+
"id": "MDU6TGFiZWw2NDQwOTU4ODI=",
263+
"name": "Unmerged",
264+
"__typename": "Label"
265+
},
256266
{
257267
"id": "MDU6TGFiZWw2NDY3ODkyMDU=",
258268
"name": "Unowned",
@@ -272,11 +282,6 @@
272282
"id": "MDU6TGFiZWwyMDk2NzQ1NzQx",
273283
"name": "Where is GH Actions?",
274284
"__typename": "Label"
275-
},
276-
{
277-
"id": "MDU6TGFiZWw2NDQwOTU4ODI=",
278-
"name": "YSYL",
279-
"__typename": "Label"
280285
}
281286
]
282287
}

src/_tests/fixtures/38979/mutations.json

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
"variables": {
55
"input": {
66
"labelIds": [
7-
"MDU6TGFiZWwxNjA4NjM0NDg0"
7+
"MDU6TGFiZWwxNjA4NjM0NDg0",
8+
"MDU6TGFiZWwyNDYyODA0MzE1"
89
],
910
"labelableId": "MDExOlB1bGxSZXF1ZXN0MzI1ODk5Njc0"
1011
}
@@ -26,7 +27,7 @@
2627
"variables": {
2728
"input": {
2829
"contentId": "MDExOlB1bGxSZXF1ZXN0MzI1ODk5Njc0",
29-
"projectColumnId": "MDEzOlByb2plY3RDb2x1bW43NTUyOTIy"
30+
"projectColumnId": "MDEzOlByb2plY3RDb2x1bW45ODY3MDA2"
3031
}
3132
}
3233
},
@@ -35,7 +36,7 @@
3536
"variables": {
3637
"input": {
3738
"id": "MDEyOklzc3VlQ29tbWVudDYyNzA5NzAxMg==",
38-
"body": "@ExE-Boss Thank you for submitting this PR!\n\n***This is a live comment which I will keep updated.***\n\n## 1 package in this PR\n\n* `es-abstract` [on npm](https://www.npmjs.com/package/es-abstract), [on unpkg](https://unpkg.com/browse/es-abstract@latest/)\n\n## Code Reviews\n\nBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged.\n\n## Status\n\n * ✅ No merge conflicts\n * ✅ Continuous integration tests have passed\n * ❌ A DT maintainer needs to approve changes which affect module config files\n - [`es-abstract/tsconfig.json`](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/38979/files/222334139e52fc16369464cfb5dc95c82f71192f#diff-abcd3ac52c6c4c77c7fa2a0e5bc09313ca1cbfd335f929838b0a4e3a607774cc): couldn't fetch contents\n - [`es-abstract/tslint.json`](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/38979/files/222334139e52fc16369464cfb5dc95c82f71192f#diff-1eda518cd7bfbcd5fa96a7f844b631954cbc9db9ff168fc3731abb874369a4f6): couldn't fetch contents\n\nOnce every item on this list is checked, I'll ask you for permission to merge and publish the changes.\n\n----------------------\n... diagnostics scrubbed ...\n<!--typescript_bot_welcome-->"
39+
"body": "@ExE-Boss Thank you for submitting this PR!\n\n***This is a live comment which I will keep updated.***\n\n## 1 package in this PR\n\n* `es-abstract` [on npm](https://www.npmjs.com/package/es-abstract), [on unpkg](https://unpkg.com/browse/es-abstract@latest/)\n\n## Code Reviews\n\nBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged.\n\n## Status\n\n * ✅ No merge conflicts\n * ✅ Continuous integration tests have passed\n * ❌ A DT maintainer needs to approve changes which affect module config files\n - [`es-abstract/tsconfig.json`](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/38979/files/222334139e52fc16369464cfb5dc95c82f71192f#diff-abcd3ac52c6c4c77c7fa2a0e5bc09313ca1cbfd335f929838b0a4e3a607774cc): couldn't fetch contents\n - [`es-abstract/tslint.json`](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/38979/files/222334139e52fc16369464cfb5dc95c82f71192f#diff-1eda518cd7bfbcd5fa96a7f844b631954cbc9db9ff168fc3731abb874369a4f6): couldn't fetch contents\n\nOnce every item on this list is checked, I'll ask you for permission to merge and publish the changes.\n\n## Inactive\n\nThis PR has been inactive for 129 days — it is *still* unreviewed!\n\n----------------------\n... diagnostics scrubbed ...\n<!--typescript_bot_welcome-->"
3940
}
4041
}
4142
},
@@ -56,5 +57,14 @@
5657
"body": "@sandersn, @weswigham Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?\n<!--typescript_bot_stale-ping-764528-0e6e7f4-->"
5758
}
5859
}
60+
},
61+
{
62+
"query": "mutation($input: AddCommentInput!) { addComment(input: $input) { clientMutationId } }",
63+
"variables": {
64+
"input": {
65+
"subjectId": "MDExOlB1bGxSZXF1ZXN0MzI1ODk5Njc0",
66+
"body": "It has been more than two weeks and this PR still has no reviews.\n\nI'll bump it to the DT maintainer queue. Thank you for your patience, @ExE-Boss.\n\n(Ping @RReverser.)\n<!--typescript_bot_Unreviewed:done-->"
67+
}
68+
}
5969
}
6070
]

src/_tests/fixtures/38979/result.json

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
{
22
"pr_number": 38979,
3-
"targetColumn": "Needs Maintainer Review",
3+
"targetColumn": "Needs Maintainer Action",
44
"labels": [
55
"Critical package",
66
"Other Approved",
7-
"Config Edit"
7+
"Config Edit",
8+
"Unreviewed"
89
],
910
"responseComments": [
1011
{
1112
"tag": "welcome",
12-
"status": "@ExE-Boss Thank you for submitting this PR!\n\n***This is a live comment which I will keep updated.***\n\n## 1 package in this PR\n\n* `es-abstract` [on npm](https://www.npmjs.com/package/es-abstract), [on unpkg](https://unpkg.com/browse/es-abstract@latest/)\n\n## Code Reviews\n\nBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged.\n\n## Status\n\n * ✅ No merge conflicts\n * ✅ Continuous integration tests have passed\n * ❌ A DT maintainer needs to approve changes which affect module config files\n - [`es-abstract/tsconfig.json`](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/38979/files/222334139e52fc16369464cfb5dc95c82f71192f#diff-abcd3ac52c6c4c77c7fa2a0e5bc09313ca1cbfd335f929838b0a4e3a607774cc): couldn't fetch contents\n - [`es-abstract/tslint.json`](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/38979/files/222334139e52fc16369464cfb5dc95c82f71192f#diff-1eda518cd7bfbcd5fa96a7f844b631954cbc9db9ff168fc3731abb874369a4f6): couldn't fetch contents\n\nOnce every item on this list is checked, I'll ask you for permission to merge and publish the changes.\n\n----------------------\n... diagnostics scrubbed ..."
13+
"status": "@ExE-Boss Thank you for submitting this PR!\n\n***This is a live comment which I will keep updated.***\n\n## 1 package in this PR\n\n* `es-abstract` [on npm](https://www.npmjs.com/package/es-abstract), [on unpkg](https://unpkg.com/browse/es-abstract@latest/)\n\n## Code Reviews\n\nBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged.\n\n## Status\n\n * ✅ No merge conflicts\n * ✅ Continuous integration tests have passed\n * ❌ A DT maintainer needs to approve changes which affect module config files\n - [`es-abstract/tsconfig.json`](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/38979/files/222334139e52fc16369464cfb5dc95c82f71192f#diff-abcd3ac52c6c4c77c7fa2a0e5bc09313ca1cbfd335f929838b0a4e3a607774cc): couldn't fetch contents\n - [`es-abstract/tslint.json`](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/38979/files/222334139e52fc16369464cfb5dc95c82f71192f#diff-1eda518cd7bfbcd5fa96a7f844b631954cbc9db9ff168fc3731abb874369a4f6): couldn't fetch contents\n\nOnce every item on this list is checked, I'll ask you for permission to merge and publish the changes.\n\n## Inactive\n\nThis PR has been inactive for 129 days — it is *still* unreviewed!\n\n----------------------\n... diagnostics scrubbed ..."
1314
},
1415
{
1516
"tag": "pinging-reviewers",
@@ -18,6 +19,10 @@
1819
{
1920
"tag": "stale-ping-764528-0e6e7f4",
2021
"status": "@sandersn, @weswigham Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?"
22+
},
23+
{
24+
"tag": "Unreviewed:done",
25+
"status": "It has been more than two weeks and this PR still has no reviews.\n\nI'll bump it to the DT maintainer queue. Thank you for your patience, @ExE-Boss.\n\n(Ping @RReverser.)"
2126
}
2227
],
2328
"shouldClose": false,

src/_tests/fixtures/43144/mutations.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
"variables": {
2525
"input": {
2626
"subjectId": "MDExOlB1bGxSZXF1ZXN0Mzg4NDkxOTI2",
27-
"body": "@jeffreymeng Thank you for submitting this PR!\n\n***This is a live comment which I will keep updated.***\n\n## 1 package in this PR\n\n* `mailcheck` [on npm](https://www.npmjs.com/package/mailcheck), [on unpkg](https://unpkg.com/browse/mailcheck@latest/)\n - owner-approval: @pocesar\n\n## Code Reviews\n\nBecause you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.\n\n## Status\n\n * ✅ No merge conflicts\n * ✅ Continuous integration tests have passed\n * ✅ Most recent commit is approved by type definition owners, DT maintainers or others\n\nAll of the items on the list are green. **To merge, you need to post a comment including the string \"Ready to merge\"** to bring in your changes.\n\n## Inactive\n\nThis PR has been inactive for 4 days.\n\n----------------------\n... diagnostics scrubbed ...\n<!--typescript_bot_welcome-->"
27+
"body": "@jeffreymeng Thank you for submitting this PR!\n\n***This is a live comment which I will keep updated.***\n\n## 1 package in this PR\n\n* `mailcheck` [on npm](https://www.npmjs.com/package/mailcheck), [on unpkg](https://unpkg.com/browse/mailcheck@latest/)\n - owner-approval: @pocesar\n\n## Code Reviews\n\nBecause you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.\n\n## Status\n\n * ✅ No merge conflicts\n * ✅ Continuous integration tests have passed\n * ✅ Most recent commit is approved by type definition owners, DT maintainers or others\n\nAll of the items on the list are green. **To merge, you need to post a comment including the string \"Ready to merge\"** to bring in your changes.\n\n----------------------\n... diagnostics scrubbed ...\n<!--typescript_bot_welcome-->"
2828
}
2929
}
3030
},

src/_tests/fixtures/43144/result.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
"responseComments": [
99
{
1010
"tag": "welcome",
11-
"status": "@jeffreymeng Thank you for submitting this PR!\n\n***This is a live comment which I will keep updated.***\n\n## 1 package in this PR\n\n* `mailcheck` [on npm](https://www.npmjs.com/package/mailcheck), [on unpkg](https://unpkg.com/browse/mailcheck@latest/)\n - owner-approval: @pocesar\n\n## Code Reviews\n\nBecause you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.\n\n## Status\n\n * ✅ No merge conflicts\n * ✅ Continuous integration tests have passed\n * ✅ Most recent commit is approved by type definition owners, DT maintainers or others\n\nAll of the items on the list are green. **To merge, you need to post a comment including the string \"Ready to merge\"** to bring in your changes.\n\n## Inactive\n\nThis PR has been inactive for 4 days.\n\n----------------------\n... diagnostics scrubbed ..."
11+
"status": "@jeffreymeng Thank you for submitting this PR!\n\n***This is a live comment which I will keep updated.***\n\n## 1 package in this PR\n\n* `mailcheck` [on npm](https://www.npmjs.com/package/mailcheck), [on unpkg](https://unpkg.com/browse/mailcheck@latest/)\n - owner-approval: @pocesar\n\n## Code Reviews\n\nBecause you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.\n\n## Status\n\n * ✅ No merge conflicts\n * ✅ Continuous integration tests have passed\n * ✅ Most recent commit is approved by type definition owners, DT maintainers or others\n\nAll of the items on the list are green. **To merge, you need to post a comment including the string \"Ready to merge\"** to bring in your changes.\n\n----------------------\n... diagnostics scrubbed ..."
1212
},
1313
{
1414
"tag": "merge-offer",

src/_tests/fixtures/43695-duplicate-comment/mutations.json

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,15 @@
11
[
2+
{
3+
"query": "mutation($input: AddLabelsToLabelableInput!) { addLabelsToLabelable(input: $input) { clientMutationId } }",
4+
"variables": {
5+
"input": {
6+
"labelIds": [
7+
"MDU6TGFiZWwyNDYyODA0MzE1"
8+
],
9+
"labelableId": "MDExOlB1bGxSZXF1ZXN0NDAwMTAwMTk2"
10+
}
11+
}
12+
},
213
{
314
"query": "mutation($input: RemoveLabelsFromLabelableInput!) { removeLabelsFromLabelable(input: $input) { clientMutationId } }",
415
"variables": {
@@ -25,7 +36,7 @@
2536
"variables": {
2637
"input": {
2738
"id": "MDEyOklzc3VlQ29tbWVudDYxMDIzNDI3MA==",
28-
"body": "@alexandercerutti Thank you for submitting this PR!\n\n***This is a live comment which I will keep updated.***\n\n## 1 package in this PR\n\n* `accedo__accedo-one` (*new!*) [on npm](https://www.npmjs.com/package/@accedo/accedo-one), [on unpkg](https://unpkg.com/browse/@accedo/accedo-one@latest/)\n - 1 added owner: ✎@alexandercerutti\n\n## Code Reviews\n\nThis PR adds a new definition, so it needs to be reviewed by a DT maintainer before it can be merged.\n\n## Status\n\n * ✅ No merge conflicts\n * ✅ Continuous integration tests have passed\n * ❌ Only a DT maintainer can approve changes when there are new packages added\n\nOnce every item on this list is checked, I'll ask you for permission to merge and publish the changes.\n\n----------------------\n... diagnostics scrubbed ...\n<!--typescript_bot_welcome-->"
39+
"body": "@alexandercerutti Thank you for submitting this PR!\n\n***This is a live comment which I will keep updated.***\n\n## 1 package in this PR\n\n* `accedo__accedo-one` (*new!*) [on npm](https://www.npmjs.com/package/@accedo/accedo-one), [on unpkg](https://unpkg.com/browse/@accedo/accedo-one@latest/)\n - 1 added owner: ✎@alexandercerutti\n\n## Code Reviews\n\nThis PR adds a new definition, so it needs to be reviewed by a DT maintainer before it can be merged.\n\n## Status\n\n * ✅ No merge conflicts\n * ✅ Continuous integration tests have passed\n * ❌ Only a DT maintainer can approve changes when there are new packages added\n\nOnce every item on this list is checked, I'll ask you for permission to merge and publish the changes.\n\n## Inactive\n\nThis PR has been inactive for 24 days — it is *still* unreviewed!\n\n----------------------\n... diagnostics scrubbed ...\n<!--typescript_bot_welcome-->"
2940
}
3041
}
3142
},
@@ -46,5 +57,14 @@
4657
"body": "@andrewbranch, @RyanCavanaugh Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?\n<!--typescript_bot_stale-ping-61f252-a5285cd-->"
4758
}
4859
}
60+
},
61+
{
62+
"query": "mutation($input: AddCommentInput!) { addComment(input: $input) { clientMutationId } }",
63+
"variables": {
64+
"input": {
65+
"subjectId": "MDExOlB1bGxSZXF1ZXN0NDAwMTAwMTk2",
66+
"body": "It has been more than two weeks and this PR still has no reviews.\n\nI'll bump it to the DT maintainer queue. Thank you for your patience, @alexandercerutti.\n\n(Ping «anyone?».)\n<!--typescript_bot_Unreviewed:done-->"
67+
}
68+
}
4969
}
5070
]

0 commit comments

Comments
 (0)