Skip to content

Commit a91b518

Browse files
authored
Add bot to ping reviewers after no activity (#9973)
* Add bot to ping reviewers after no activity * Address comments Co-authored-by: driazati <[email protected]>
1 parent 8727c60 commit a91b518

File tree

4 files changed

+394
-1
lines changed

4 files changed

+394
-1
lines changed
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
name: Ping Reviewers
2+
on:
3+
schedule:
4+
- cron: "0/15 * * * *"
5+
workflow_dispatch:
6+
7+
concurrency:
8+
group: ping
9+
cancel-in-progress: true
10+
11+
jobs:
12+
ping:
13+
runs-on: ubuntu-20.04
14+
steps:
15+
- uses: actions/checkout@v2
16+
- name: Ping reviewers
17+
env:
18+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
19+
run: |
20+
set -eux
21+
python tests/scripts/ping_reviewers.py --wait-time-minutes 1440

docs/contribute/code_review.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,3 +220,11 @@ To do so -- please click on changes tab in the pull request, then select approve
220220
or comment on the code and click request changes.
221221
Code owner can decide if the code can be merged in case by case if some of the reviewers
222222
did not respond in time(e.g. a week) and existing reviews are sufficient.
223+
224+
Reviewers
225+
~~~~~~~~~
226+
Reviewers should strive to leave timely feedback on pull requests for which their
227+
review was requested. Reviewing code is an important part of the project's health
228+
and should be considered a regular responsibility for contributors. Automated
229+
tooling helps out in this regard, as PRs with no activity for a set amount of
230+
time will get a bot comment pinging the relevant parties.

tests/python/unittest/test_ci.py

Lines changed: 133 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def test(commands, should_skip, pr_title, why):
8484
git.run(*command)
8585
pr_number = "1234"
8686
proc = subprocess.run(
87-
[str(skip_ci_script), "--pr", pr_number, "--pr-title", pr_title], cwd=git.cwd # dir
87+
[str(skip_ci_script), "--pr", pr_number, "--pr-title", pr_title], cwd=git.cwd
8888
)
8989
expected = 0 if should_skip else 1
9090
assert proc.returncode == expected, why
@@ -161,5 +161,137 @@ def test(commands, should_skip, pr_title, why):
161161
)
162162

163163

164+
def test_ping_reviewers(tmpdir_factory):
165+
reviewers_script = REPO_ROOT / "tests" / "scripts" / "ping_reviewers.py"
166+
167+
def run(pr, check):
168+
git = TempGit(tmpdir_factory.mktemp("tmp_git_dir"))
169+
# Jenkins git is too old and doesn't have 'git init --initial-branch'
170+
git.run("init")
171+
git.run("checkout", "-b", "main")
172+
git.run("remote", "add", "origin", "https://github.com/apache/tvm.git")
173+
174+
data = {
175+
"data": {
176+
"repository": {
177+
"pullRequests": {
178+
"nodes": [pr],
179+
"edges": [],
180+
}
181+
}
182+
}
183+
}
184+
proc = subprocess.run(
185+
[
186+
str(reviewers_script),
187+
"--dry-run",
188+
"--wait-time-minutes",
189+
"1",
190+
"--cutoff-pr-number",
191+
"5",
192+
"--allowlist",
193+
"user",
194+
"--pr-json",
195+
json.dumps(data),
196+
"--now",
197+
"2022-01-26T17:54:19Z",
198+
],
199+
stdout=subprocess.PIPE,
200+
stderr=subprocess.PIPE,
201+
encoding="utf-8",
202+
cwd=git.cwd,
203+
)
204+
if proc.returncode != 0:
205+
raise RuntimeError(f"Process failed:\nstdout:\n{proc.stdout}\n\nstderr:\n{proc.stderr}")
206+
207+
assert check in proc.stdout
208+
209+
run(
210+
{
211+
"isDraft": True,
212+
},
213+
"Checking 0 PRs",
214+
)
215+
216+
run(
217+
{
218+
"isDraft": False,
219+
"number": 2,
220+
},
221+
"Checking 0 PRs",
222+
)
223+
224+
run(
225+
{
226+
"number": 123,
227+
"url": "https://github.com/apache/tvm/pull/123",
228+
"body": "cc @someone",
229+
"isDraft": False,
230+
"author": {"login": "user"},
231+
"reviews": {"nodes": []},
232+
"publishedAt": "2022-01-18T17:54:19Z",
233+
"comments": {"nodes": []},
234+
},
235+
"Pinging reviewers ['someone'] on https://github.com/apache/tvm/pull/123",
236+
)
237+
238+
# Check allowlist functionality
239+
run(
240+
{
241+
"number": 123,
242+
"url": "https://github.com/apache/tvm/pull/123",
243+
"body": "cc @someone",
244+
"isDraft": False,
245+
"author": {"login": "user2"},
246+
"reviews": {"nodes": []},
247+
"publishedAt": "2022-01-18T17:54:19Z",
248+
"comments": {
249+
"nodes": [
250+
{"updatedAt": "2022-01-19T17:54:19Z", "bodyText": "abc"},
251+
]
252+
},
253+
},
254+
"Checking 0 PRs",
255+
)
256+
257+
# Old comment, ping
258+
run(
259+
{
260+
"number": 123,
261+
"url": "https://github.com/apache/tvm/pull/123",
262+
"body": "cc @someone",
263+
"isDraft": False,
264+
"author": {"login": "user"},
265+
"reviews": {"nodes": []},
266+
"publishedAt": "2022-01-18T17:54:19Z",
267+
"comments": {
268+
"nodes": [
269+
{"updatedAt": "2022-01-19T17:54:19Z", "bodyText": "abc"},
270+
]
271+
},
272+
},
273+
"Pinging reviewers ['someone'] on https://github.com/apache/tvm/pull/123",
274+
)
275+
276+
# New comment, don't ping
277+
run(
278+
{
279+
"number": 123,
280+
"url": "https://github.com/apache/tvm/pull/123",
281+
"body": "cc @someone",
282+
"isDraft": False,
283+
"author": {"login": "user"},
284+
"reviews": {"nodes": []},
285+
"publishedAt": "2022-01-18T17:54:19Z",
286+
"comments": {
287+
"nodes": [
288+
{"updatedAt": "2022-01-27T17:54:19Z", "bodyText": "abc"},
289+
]
290+
},
291+
},
292+
"Not pinging PR 123",
293+
)
294+
295+
164296
if __name__ == "__main__":
165297
sys.exit(pytest.main([__file__] + sys.argv[1:]))

0 commit comments

Comments
 (0)