-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ci] Add bot to post welcome comment #12695
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @driazati, couple of questions here.
cc @gigiblender can you have a look?
|
||
from git_utils import GitHubRepo | ||
|
||
BOT_COMMENT_START = "<!---bot-comment-->" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does this related to bot-comment-foo-start?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this marks the start of the whole comment, the -foo-start
marks the boundaries of sections within the comment
raise RuntimeError( | ||
f"Malformed comment found: {start} marker did not have matching end, found instead {end}" | ||
) | ||
items[start] = text.strip().lstrip("* ") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why lstrip the *
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so that later on the existing items extracted here from github can be treated the same way as newly produced ones (pretty much the *
is stripped off here and added back there)
def _post_comment(self, body_items: Dict[str, str]): | ||
comment = BOT_COMMENT_START + "\n\n" + WELCOME_TEXT + "\n\n" | ||
for key, content in body_items.items(): | ||
line = self.start_key(key) + "\n * " + content.strip() + self.end_key(key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we textutil.indent(content.strip())? what if it's multi-line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
multiline should be fine (and needs to be since the tests-bot uses it) since GitHub will render it and the markers define the edges, not newlines
docs_info = get_doc_url(pr_data) | ||
|
||
items = { | ||
"ccs": ccs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to also keep any sorta central registry of these? could conceptually put these keys as an enum in/next to BotCommentBuilder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is kind of the central registry already, nothing else outside this script really needs to know the specifics of what the sections are since its all generic
@tvm-bot rerun |
It has been a while since this PR was updated, @areusch @Mousius @gigiblender please leave a review or address the outstanding comments. @driazati if this PR is still a work in progress, please convert it to a draft until it is ready for review. |
This implements apache#12781 but needs to wait for apache#12695 before it can merge
This would post the comment that the tests bot and the docs comment bot uses straightaway when a PR is posted. This will contain links to generic info about posting PRs (and obviate the `.github/PULL_REQUEST_TEMPLATE.md`) as well as dynamic info about the specific PR (filled in later by the respective bots). This would make things like the auto-cc bot more transparent since it would have a link to the relevant issue.
Thanks @driazati! LGTM |
This implements apache#12781 but needs to wait for apache#12695 before it can merge
This implements apache#12781 but needs to wait for apache#12695 before it can merge
This implements apache#12781 but needs to wait for apache#12695 before it can merge
@areusch @driazati @Mousius Please consider reverting these changes. In apache/pulsar we have been experiencing slowness for GitHub Actions since there aren't available runners. There's over 1000 workflow runs in your queue: https://github.com/apache/tvm/actions?query=is%3Aqueued and there's also 135 in progress: https://github.com/apache/tvm/actions?query=is%3Ain_progress . It seems that a lot of the ones shown in progress have been cancelled and that's probably a GitHub Actions bug ( related report community/community#31690 ). |
This would post the comment that the tests bot and the docs comment bot uses straightaway when a PR is posted. This will contain links to generic info about posting PRs (and obviate the `.github/PULL_REQUEST_TEMPLATE.md`) as well as dynamic info about the specific PR (filled in later by the respective bots). This would make things like the auto-cc bot more transparent since it would have a link to the relevant issue. Tested live here: driazati#21 (comment)
This would post the comment that the tests bot and the docs comment bot
uses straightaway when a PR is posted. This will contain links to
generic info about posting PRs (and obviate the
.github/PULL_REQUEST_TEMPLATE.md
) as well as dynamic info about thespecific PR (filled in later by the respective bots). This would make
things like the auto-cc bot more transparent since it would have a link
to the relevant issue.
Tested live here: driazati#21 (comment)
cc @Mousius @areusch @gigiblender