Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[ci] Add GitHub Actions bot to merge PRs on demand #10833

Merged
merged 4 commits into from
May 24, 2022
Merged

[ci] Add GitHub Actions bot to merge PRs on demand #10833

merged 4 commits into from
May 24, 2022

Conversation

driazati
Copy link
Member

@driazati driazati commented Mar 30, 2022 •
edited
Loading

This implements https://discuss.tvm.apache.org/t/rfc-allow-merging-via-pr-comments/12220. The bot can be invoked from a top-level review comment or via a regular PR comment. The text @tvm-bot merge anywhere in the body will trigger the bot. Right now it checks that the latest commit is reviewed and that all CI jobs that have run on that commit are successful. If it fails, it will leave a comment on the PR with the reason.

This is just a start and some features are left for followups:

  • Various TODOs throughout the code
  • "Scheduled" merges that happen once CI finishes
  • Allowing committers to merge without getting a fresh review for changes after an approval

Test PRs

Description Link
ci passed: no, then yes
approved: no, then yes		 driazati#13
ci passed: yes
approved: yes, then PR is updated, then re-approved		 driazati#17
commit with a message driazati#16
cancelled merge driazati#18

cc @areusch @Mousius @gromero @kparzysz-quic

@driazati driazati force-pushed the mergebot branch 8 times, most recently from ea2edd9 to a47c9f5 Compare March 31, 2022 22:24
@driazati driazati marked this pull request as ready for review April 11, 2022 21:50
@gromero
Copy link
Contributor

gromero commented Apr 11, 2022 •
edited
Loading

@driazati Thanks!

An initial question: for both examples driazati#13 and driazati#17, it seems that the final commit messages that landed were not taken from the PR title/body (are empty?), but that's planned tho (WIP)?

@driazati
Copy link
Member Author

driazati commented Apr 11, 2022 •
edited
Loading

Ah I could have used better examples, the code strips out the specific text Thanks for contributing to TVM! ... since that is just for PR authors and shouldn't really be in the commit log.

edit: see driazati#16 and driazati@80acd9f

@gromero
Copy link
Contributor

gromero commented Apr 12, 2022

hmm neat :)

areusch
areusch reviewed Apr 12, 2022
View reviewed changes
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @driazati couple comments

tests/python/ci/sample_prs/pr10786-badci.json Show resolved Hide resolved
tests/python/ci/sample_prs/pr10786-badci.json Show resolved Hide resolved
tests/python/ci/sample_prs/pr10786-oldreview.json Outdated Show resolved Hide resolved
tests/scripts/github_mergebot.py Show resolved Hide resolved
tests/scripts/github_mergebot.py Show resolved Hide resolved
@driazati driazati requested a review from areusch April 18, 2022 19:26
@areusch
Copy link
Contributor

areusch commented Apr 19, 2022

this one ready for a review?

@driazati
Copy link
Member Author

yup it's ready for review, let me know if there's any more test cases you'd want to see as well

areusch
areusch reviewed Apr 19, 2022
View reviewed changes
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool this is looking pretty good. a few more questions, but i think we're getting close here.

.github/workflows/merge.yml Outdated Show resolved Hide resolved
tests/python/ci/sample_prs/pr10786-badci.json Show resolved Hide resolved
tests/scripts/github_mergebot.py Show resolved Hide resolved
tests/scripts/github_mergebot.py Outdated Show resolved Hide resolved
@driazati driazati requested a review from areusch April 25, 2022 20:59
@driazati driazati force-pushed the mergebot branch 2 times, most recently from 1391d5d to 8427b3e Compare April 27, 2022 21:59
areusch
areusch approved these changes Apr 27, 2022
View reviewed changes
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @driazati !

@github-actions
Copy link
Contributor

github-actions bot commented May 4, 2022

It has been a while since this PR was updated, @Mousius @kparzysz-quic @areusch @gromero 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.

Mousius
Mousius requested changes May 5, 2022
View reviewed changes
tests/scripts/github_mergebot.py Outdated Show resolved Hide resolved
tests/scripts/github_mergebot.py Outdated Show resolved Hide resolved
tests/scripts/github_mergebot.py Outdated Show resolved Hide resolved
tests/scripts/github_mergebot.py Outdated Show resolved Hide resolved
tests/scripts/github_mergebot.py Outdated Show resolved Hide resolved
tests/scripts/github_mergebot.py Show resolved Hide resolved
tests/scripts/github_mergebot.py Show resolved Hide resolved
tests/scripts/github_mergebot.py Show resolved Hide resolved
tests/scripts/github_mergebot.py Outdated Show resolved Hide resolved
tests/scripts/github_mergebot.py Outdated Show resolved Hide resolved
@driazati
Copy link
Member Author

@Mousius can you re-review?

Mousius
Mousius requested changes May 19, 2022
View reviewed changes
Copy link
Member

@Mousius Mousius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments left @driazati, otherwise I think this is getting pretty close 😸

tests/scripts/github_mergebot.py Show resolved Hide resolved
tests/scripts/github_mergebot.py Outdated Show resolved Hide resolved
tests/scripts/github_mergebot.py Outdated Show resolved Hide resolved
tests/scripts/github_mergebot.py Outdated Show resolved Hide resolved
@driazati
[ci] Add GitHub Actions bot to merge PRs on demand
7c10ee6 
This implements https://discuss.tvm.apache.org/t/rfc-allow-merging-via-pr-comments/12220. The bot can be invoked from a top-level review comment or via a regular PR comment. The text `@tvm-bot merge` anywhere in the body will trigger the bot. Right now it checks that the latest commit is reviewed and that all CI jobs that have run on that commit are successful. If it fails, it will leave a comment on the PR with the reason.

This is just a start and some features are left for followups:
* Various TODOs throughout the code
* "Scheduled" merges that happen once CI finishes
* Allowing committers to merge without getting a fresh review for changes after an approval
@driazati
Address comments
5d4dbc0
@driazati
Address comments, add author filter
f49c19c
@driazati
Address comments: test cases, code cleanups
9b74a5a
@Mousius
Copy link
Member

Mousius commented May 24, 2022

Apologies for the delay @driazati , this LGTM and I'm glad to see this work finally land 😸

@Mousius Mousius merged commit 59d4c02 into apache:main May 24, 2022
@driazati driazati mentioned this pull request Jul 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@areusch areusch areusch approved these changes

@Mousius Mousius Mousius approved these changes

@gromero gromero Awaiting requested review from gromero

@kparzysz-quic kparzysz-quic Awaiting requested review from kparzysz-quic

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@driazati @gromero @areusch @Mousius