Skip to content

Commit

Permalink
[ci] Clean up mergebot commit messages (#11437)
Browse files Browse the repository at this point in the history
* [ci] Clean up mergebot commit messages

Adds both bullets and closes #11433

* Fix error from pr #11442

Co-authored-by: driazati <driazati@users.noreply.github.com>
  • Loading branch information
driazati and driazati authored May 26, 2022
1 parent 2f21698 commit cfcca59
Show file tree
Hide file tree
Showing 4 changed files with 221 additions and 9 deletions.
2 changes: 1 addition & 1 deletion tests/python/ci/sample_prs/pr10786-merges.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"title": "[Hexagon] 2-d allocation cleanup",
"body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw <astraw@octoml.ai>",
"body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free.\n\n\nThanks 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.\n\n\nPreviously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\n\n\ncc @someone\n\r\n\r\nCo-authored-by: Adam Straw <astraw@octoml.ai>\n\n\nThanks 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.\n\n",
"state": "OPEN",
"author": {
"login": "Lunderberg"
Expand Down
191 changes: 191 additions & 0 deletions tests/python/ci/sample_prs/pr11442-no-recomment.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
{
"title": "Add 'static_library' runtime::Module",
"body": "(See https://discuss.tvm.apache.org/t/byoc-supporting-cutlass-byoc-with-collage/12796/6 for\r\ncontext, which in turn is part of Collage (https://github.com/apache/tvm-rfcs/blob/main/rfcs/0062-collage.md).\r\n\r\nThis adds a new 'DSO exportable' runtime module representing the contents of a .o file. It\r\nallows external codegen toolchains to yield a result which:\r\n - Like CSource modules, can be conveyed directly to the final export_library compilation\r\n step for linking into the final .so and saved to a know location without risk the\r\n underlying code artifact will be lost.\r\n - Like DSOLibrary modules, are self contained so that no additional compile-time arguments\r\n need be conveyed from the CSource module to the final export_library command line\r\n\r\nSince this is the third flavor of 'DSO exportable' module, add a Module::IsDSOExportable.\r\n\r\nSince adding the above, can't resist also adding a Module::ImplementsFunction virtual and\r\ncalling it from TEComplier to check if an external codegen function actually provided the\r\nimplementation it promised.\r\n\r\nNote:\r\n - I've left the existing implementation of runtime.load_module alone which\r\n relinks .o files to .so files.\r\n - Though also contained in the .o metadata, I require static libraries to always\r\n carry their list of exported function names.\r\n\r\nThis is all pretty stop gap pending a good rework of TVM to supoprt the notion of artifacts\r\nand, perhaps, build rules.\r\n",
"state": "OPEN",
"author": {
"login": "mbs-octoml"
},
"comments": {
"pageInfo": {
"hasPreviousPage": false
},
"nodes": [
{
"authorAssociation": "MEMBER",
"author": {
"login": "tqchen"
},
"updatedAt": "2022-05-24T22:13:29Z",
"body": "Thanks @mbs-octoml . I think we go with this as a temp workaround with a mind that the IsDSOExportable and ImplementsFunction likely should go to Artifact."
},
{
"authorAssociation": "CONTRIBUTOR",
"author": {
"login": "mbs-octoml"
},
"updatedAt": "2022-05-24T22:56:07Z",
"body": "Yeah, we really need to put some love into that.\r\n\r\nCollecting all the pieces needed for deployment along with their metadata a la Artifact is pretty clearly needed, though I suspect that will need to be abstract to cover the spectrum from firmware image to dynamically loadable .so to ready-to-call JITed code to tar.\r\n\r\nI can't help thinking we should also think about build rules guarded by target kinds & attributes, since again there's just so may ways to proceed."
},
{
"authorAssociation": "MEMBER",
"author": {
"login": "tqchen"
},
"updatedAt": "2022-05-24T23:08:00Z",
"body": "Perhaps we will end up building our own cmake/bazel :p in another time"
},
{
"authorAssociation": "CONTRIBUTOR",
"author": {
"login": "mbs-octoml"
},
"updatedAt": "2022-05-25T22:11:44Z",
"body": "Thanks Tianqi. Let's see if this new fancy bot works...\r\n\r\n"
},
{
"authorAssociation": "CONTRIBUTOR",
"author": {
"login": "mbs-octoml"
},
"updatedAt": "2022-05-25T22:11:50Z",
"body": "@tvm-bot merge"
},
{
"authorAssociation": "NONE",
"author": {
"login": "github-actions"
},
"updatedAt": "2022-05-25T22:12:10Z",
"body": "Cannot merge, did not find any approving reviews from users with write access on 96d4e62da5a7b78da18d0ee28cc6261d8fbf31c4"
},
{
"authorAssociation": "CONTRIBUTOR",
"author": {
"login": "mbs-octoml"
},
"updatedAt": "2022-05-25T22:12:37Z",
"body": "Hmff."
},
{
"authorAssociation": "NONE",
"author": {
"login": "github-actions"
},
"updatedAt": "2022-05-25T22:12:55Z",
"body": "Cannot merge, did not find any approving reviews from users with write access on 96d4e62da5a7b78da18d0ee28cc6261d8fbf31c4"
}
]
},
"authorCommits": {
"nodes": [
{
"commit": {
"authors": {
"nodes": [
{
"name": "mbs-octoml",
"email": "mbs@octoml.ai"
}
]
}
}
}
]
},
"commits": {
"nodes": [
{
"commit": {
"oid": "96d4e62da5a7b78da18d0ee28cc6261d8fbf31c4",
"statusCheckRollup": {
"contexts": {
"pageInfo": {
"hasNextPage": false
},
"nodes": [
{
"name": "MacOS",
"checkSuite": {
"workflowRun": {
"workflow": {
"name": "CI"
}
}
},
"status": "COMPLETED",
"conclusion": "SUCCESS",
"url": "https://github.com/apache/tvm/runs/6598275844"
},
{
"name": "cc-reviewers",
"checkSuite": {
"workflowRun": {
"workflow": {
"name": "PR"
}
}
},
"status": "COMPLETED",
"conclusion": "SUCCESS",
"url": "https://github.com/apache/tvm/runs/6598273162"
},
{
"name": "Windows",
"checkSuite": {
"workflowRun": {
"workflow": {
"name": "CI"
}
}
},
"status": "COMPLETED",
"conclusion": "SUCCESS",
"url": "https://github.com/apache/tvm/runs/6598275717"
},
{
"name": "Android",
"checkSuite": {
"workflowRun": {
"workflow": {
"name": "CI"
}
}
},
"status": "COMPLETED",
"conclusion": "SUCCESS",
"url": "https://github.com/apache/tvm/runs/6598275593"
},
{
"state": "SUCCESS",
"context": "tvm-ci/pr-head",
"targetUrl": "https://ci.tlcpack.ai/job/tvm/job/PR-11442/4/display/redirect"
}
]
}
}
}
}
]
},
"reviewDecision": "APPROVED",
"reviews": {
"pageInfo": {
"hasPreviousPage": false
},
"nodes": [
{
"body": "",
"updatedAt": "2022-05-24T23:08:31Z",
"url": "https://github.com/apache/tvm/pull/11442#pullrequestreview-983954561",
"authorCanPushToRepository": true,
"commit": {
"oid": "23c600097cf1c2a55acda059626a060e106dd023"
},
"author": {
"login": "tqchen"
},
"state": "APPROVED"
}
]
}
}
18 changes: 17 additions & 1 deletion tests/python/ci/test_mergebot.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,21 @@ def run(self, *args):
raise RuntimeError(f"git command failed: '{args}'")


SUCCESS_EXPECTED_OUTPUT = """
Dry run, would have merged with url=pulls/10786/merge and data={
"commit_title": "[Hexagon] 2-d allocation cleanup (#10786)",
"commit_message": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\\n\\n- Check for \\"global.vtcm\\" scope instead of \\"vtcm\\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\\"global.vtcm\\"`. The previous check allowed unsupported scope such as `\\"local.vtcm\\"`.\\n\\n- Remove `vtcmallocs` entry after calling free.\\n\\nPreviously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\\n\\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\\n\\n\\ncc someone\\n\\n\\nCo-authored-by: Adam Straw <astraw@octoml.ai>",
"sha": "6f04bcf57d07f915a98fd91178f04d9e92a09fcd",
"merge_method": "squash"
}
""".strip()


test_data = {
"successful-merge": {
"number": 10786,
"filename": "pr10786-merges.json",
"expected": "Dry run, would have merged with url=pulls/10786/merge",
"expected": SUCCESS_EXPECTED_OUTPUT,
"detail": "Everything is fine so this PR will merge",
},
"no-request": {
Expand Down Expand Up @@ -96,6 +106,12 @@ def run(self, *args):
"expected": "Co-authored-by: Some One <someone@email.com>",
"detail": "Check that a merge request with co-authors generates the correct commit message",
},
"no-recomment": {
"number": 11442,
"filename": "pr11442-no-recomment.json",
"expected": "No merge requested, exiting",
"detail": "Check that comments after a failed merge don't trigger another merge",
},
}


Expand Down
19 changes: 12 additions & 7 deletions tests/scripts/github_mergebot.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import warnings
import logging
import traceback
import re
from typing import Dict, Any, List, Optional
from pathlib import Path

Expand All @@ -33,7 +34,7 @@
CIJob = Dict[str, Any]

EXPECTED_JOBS = ["tvm-ci/pr-head"]
THANKS_MESSAGE = "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."
THANKS_MESSAGE = r"(\s*)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.(\s*)"


def to_json_str(obj: Any) -> str:
Expand Down Expand Up @@ -315,11 +316,12 @@ def state(self) -> str:

def processed_body(self) -> str:
body = self.raw["body"].strip().replace("\r", "")
body = body.replace(
THANKS_MESSAGE,
"",
)
return body
# Remove any @-mentions of people
body = re.sub(r"(\s)@", "\g<1>", body)

# Remove the auto-inserted text since it's not useful to have in the commit log
body = re.sub(THANKS_MESSAGE, "\n\n", body)
return body.strip()

def body_with_co_authors(self) -> str:
"""
Expand Down Expand Up @@ -350,7 +352,7 @@ def merge(self) -> None:
"""
url = f"pulls/{self.number}/merge"

title = self.raw["title"]
title = self.raw["title"] + f" (#{self.number})"
body = self.body_with_co_authors()
logging.info(f"Full commit:\n{title}\n\n{body}")

Expand Down Expand Up @@ -406,6 +408,9 @@ def merge_requested(self) -> bool:
]

def parse_action(comment: Dict[str, Any]) -> Optional[str]:
if comment["author"]["login"] == "github-actions":
return "commented"

if not self.comment_can_merge(comment):
return None

Expand Down

0 comments on commit cfcca59

Please sign in to comment.