You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/02/10 03:56:11 UTC
[GitHub] [tvm] driazati opened a new pull request #10208: [ci] Check more events before pinging reviewers
driazati opened a new pull request #10208:
URL: https://github.com/apache/tvm/pull/10208
This was missing some events before (reviews without comments, PR updated from a draft -> ready for review) so these were being ignored when finding the latest event. This PR adds them and restructures the code a bit to make it more clear what is happening for each PR. This addresses some of the issues from #9983
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a change in pull request #10208: [ci] Check more events before pinging reviewers
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #10208:
URL: https://github.com/apache/tvm/pull/10208#discussion_r814205035
##########
File path: tests/scripts/ping_reviewers.py
##########
@@ -206,18 +227,27 @@ def ping_reviewers(pr, reviewers):
prs = r["data"]["repository"]["pullRequests"]["nodes"]
# Don't look at draft PRs at all
- prs = [pr for pr in prs if not pr["isDraft"]]
-
- # Don't look at super old PRs
- prs = [pr for pr in prs if pr["number"] > cutoff_pr_number]
-
- # [slow rollout]
- prs = [pr for pr in prs if pr["author"]["login"] in author_allowlist]
-
- print(f"Checking {len(prs)} PRs: {[pr['number'] for pr in prs]}")
+ prs_to_check = []
+ for pr in prs:
+ if pr["isDraft"]:
+ print(f"Skipping #{pr['number']} since it's a draft")
+ elif pr["number"] <= cutoff_pr_number:
+ print(
+ f"Skipping #{pr['number']} since it's too old ({pr['number']} <= {cutoff_pr_number})"
+ )
+ elif pr["author"]["login"] not in author_allowlist:
+ # [slow rollout]
+ print(
+ f"Skipping #{pr['number']} since author {pr['author']['login']} is not in allowlist: {author_allowlist}"
+ )
+ else:
+ print(f"Checking #{pr['number']}, {author_allowlist}")
+ prs_to_check.append(pr)
+
+ print(f"Summary: Checking {len(prs_to_check)} of {len(prs)} fetched")
Review comment:
```suggestion
print(f"Summary: Need to check {len(prs_to_check)} of {len(prs)} fetched")
```
##########
File path: tests/scripts/ping_reviewers.py
##########
@@ -206,18 +227,27 @@ def ping_reviewers(pr, reviewers):
prs = r["data"]["repository"]["pullRequests"]["nodes"]
# Don't look at draft PRs at all
- prs = [pr for pr in prs if not pr["isDraft"]]
-
- # Don't look at super old PRs
- prs = [pr for pr in prs if pr["number"] > cutoff_pr_number]
-
- # [slow rollout]
- prs = [pr for pr in prs if pr["author"]["login"] in author_allowlist]
-
- print(f"Checking {len(prs)} PRs: {[pr['number'] for pr in prs]}")
+ prs_to_check = []
+ for pr in prs:
+ if pr["isDraft"]:
+ print(f"Skipping #{pr['number']} since it's a draft")
+ elif pr["number"] <= cutoff_pr_number:
+ print(
+ f"Skipping #{pr['number']} since it's too old ({pr['number']} <= {cutoff_pr_number})"
+ )
+ elif pr["author"]["login"] not in author_allowlist:
+ # [slow rollout]
+ print(
+ f"Skipping #{pr['number']} since author {pr['author']['login']} is not in allowlist: {author_allowlist}"
+ )
+ else:
+ print(f"Checking #{pr['number']}, {author_allowlist}")
Review comment:
```suggestion
print(f"Will check #{pr['number']}, {author_allowlist}")
```
##########
File path: tests/scripts/ping_reviewers.py
##########
@@ -128,17 +147,19 @@ def check_pr(pr, wait_time, now):
if time_since_last_action > wait_time:
print(
- " Pinging reviewers",
+ " Pinging reviewers",
reviewers,
"on",
pr["url"],
"since it has been",
time_since_last_action,
- "since anything happened on that PR",
+ f"since anything happened on that PR (last action: {last_action[1]})",
)
return reviewers
else:
- print(f"Not pinging PR {pr['number']}")
+ print(
+ f" Not pinging PR {pr['number']} since it has been only {time_since_last_action} since the last action: {last_action[1]}"
Review comment:
could you add units to time print?
##########
File path: tests/python/unittest/test_ci.py
##########
@@ -244,14 +253,14 @@ def run(pr, check):
"isDraft": False,
"author": {"login": "user2"},
"reviews": {"nodes": []},
- "publishedAt": "2022-01-18T17:54:19Z",
+ **all_time_keys("2022-01-18T17:54:19Z"),
"comments": {
"nodes": [
- {"updatedAt": "2022-01-19T17:54:19Z", "bodyText": "abc"},
+ {**all_time_keys("2022-01-19T17:54:19Z"), "bodyText": "abc"},
Review comment:
TIL this syntax `{**kwargs}` was possible!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] driazati commented on a change in pull request #10208: [ci] Check more events before pinging reviewers
Posted by GitBox <gi...@apache.org>.
driazati commented on a change in pull request #10208:
URL: https://github.com/apache/tvm/pull/10208#discussion_r814209612
##########
File path: tests/scripts/ping_reviewers.py
##########
@@ -128,17 +147,19 @@ def check_pr(pr, wait_time, now):
if time_since_last_action > wait_time:
print(
- " Pinging reviewers",
+ " Pinging reviewers",
reviewers,
"on",
pr["url"],
"since it has been",
time_since_last_action,
- "since anything happened on that PR",
+ f"since anything happened on that PR (last action: {last_action[1]})",
)
return reviewers
else:
- print(f"Not pinging PR {pr['number']}")
+ print(
+ f" Not pinging PR {pr['number']} since it has been only {time_since_last_action} since the last action: {last_action[1]}"
Review comment:
it’s a formatted time so something like hh:mm:ss (so there’s not really a unit to add)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch merged pull request #10208: [ci] Check more events before pinging reviewers
Posted by GitBox <gi...@apache.org>.
areusch merged pull request #10208:
URL: https://github.com/apache/tvm/pull/10208
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on pull request #10208: [ci] Check more events before pinging reviewers
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #10208:
URL: https://github.com/apache/tvm/pull/10208#issuecomment-1050254167
we discussed offline and @driazati will address the remaining two comments in a follow-on to avoid blocking on CI test time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org