-
Notifications
You must be signed in to change notification settings - Fork 53
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
Add auto-approve and auto merge into automatic update action #364
Conversation
24f309a
to
5139a74
Compare
uses: hmarr/auto-approve-action@v3 | ||
with: | ||
pull-request-number: ${{ steps.create_pr.outputs.pull-request-number }} | ||
github-token: ${{ secrets.GITHUB_TOKEN }} |
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.
shouldn't this be token: ${{ steps.generate_token.outputs.token }}
as above?
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.
no, it should not. Because if we are using the same token it thinks that it approves PR from itself. And it is not possible
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.
I'm probably missing something but how does this auto merge the PR?
I think you can skip the auto approve completely, as it doesn't make much of a difference. The bot user has the power to skip approvals and just merge it directly once the checks are green. You would probably need something like https://github.com/re-actors/alls-green to ensure checks are actually green before merging. A couple of examples on how to do it: https://jhale.dev/posts/auto-merging-prs/ |
Co-authored-by: Jack Westbrook <[email protected]>
@jackw Auto-merge option is already in github and it is enabled for this repo. When all the requirements are there - it will be automatically merged. I thought we need only approval, but I see that we actually need something else instead. @tolzhabayev shared the example |
I think auto merge is not actually auto merge but an option you have to click on while the CI pipeline is running so that it will be merged once its done. But if you do not "click the button" it will not actually merge anything - e.g. this PR is not automatically merged although approved and all checks are green. |
@tolzhabayev yeah, that's why I was thinking auto-approve will help, but seems like no and we need to use auto-merge action to do so |
@Ukochka It seems we already have an action here that other repos are using to achieve this auto approve n merge of dependabot PRs? |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This reverts commit 5f48a18.
@tolzhabayev @jackw Hey guys, I have added merge part to this github actions. And I left approval part just to be sure. I think all-green is not needed here, because auto-merge anyway checks if everything is passed as I understand |
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.
Great work on this! Looks good 🙌🏻
With that said I'm not sure I think this feature is a good idea. Isn't it enough magic to get an PR with a updated create-plugin repository. If I would have this running in my repository I would disable this feature because I would like to be in control of when things like this gets merged.
@mckn I'm under the impression this action is intended only for the examples repo and isn't for plugin developers to use in their repos. |
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.
Alright let's see this in action
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.
LGTM! 🚀
@Ukochka fixed the CLA check #364 (comment) |
Add ability to auto approve PR created by grafana-plugin bot to be able to automerge