Skip to content

Commit

Permalink
Fix error from pr #11442
Browse files Browse the repository at this point in the history
  • Loading branch information
@driazati
driazati committed May 25, 2022
1 parent acb0de0 commit b1725d8
Show file tree
Hide file tree
Showing 3 changed files with 201 additions and 1 deletion.
191 changes: 191 additions & 0 deletions tests/python/ci/sample_prs/pr11442-no-recomment.json
View file
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"
}
]
}
}
6 changes: 6 additions & 0 deletions tests/python/ci/test_mergebot.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,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
5 changes: 4 additions & 1 deletion tests/scripts/github_mergebot.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,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 All @@ -417,7 +420,7 @@ def parse_action(comment: Dict[str, Any]) -> Optional[str]:

if any(f"@tvm-bot {c}" in body for c in cancel_commands):
return "cancel"

return None

# Check regular comments and top-level review comments
Expand Down

0 comments on commit b1725d8

Please sign in to comment.