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.

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

feat: add autoupdate-workflow #262

Merged
merged 20 commits into from
Apr 18, 2024
Merged

Conversation

Shurtu-gal
Copy link
Contributor

@Shurtu-gal Shurtu-gal commented Nov 21, 2023

Description

  • Replaces previous auto-update workflow with the current one.
  • Uses an action developed by me :- https://github.com/marketplace/actions/autoupdate-fork-action
  • The action currently runs on push to branches or /update comment and updates PRs pointing to it and having autoupdate label in it.
  • Update /help to have /update in it.

Related issue(s)
Fixes #227

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

I'm not 100% sure how it works, also please share examples where you tested it.

I'm confused because:

  • I do not understand why autoupdate workflow had to be removed
  • what is the difference between update and autoupdate (cause you left autoupdate). Is it that autoupdate you use once and then it is always triggered, and update you need to pass every time you wanna trigger update?
  • in short explain why previous action did not work and what technical change you do in your action that now bot token will be able to push to fork

@Shurtu-gal
Copy link
Contributor Author

@derberg as was discussed over slack, to not consume too much action minutes right now auto-update is only enabled for comments like \u or /update. Hence there is no need of filtering based on labels.

Successful run The related PR

Labels were meant for filtering in case of mass updates, like in this case

Also, I had removed the previous one as I this one was sufficient for both tasks, but before we do so we have to consider amount of action minutes it will take. Furthermore, GH API limit may also be reached.

Why wasn't the previous one working?

Because the API used by it requires admin access for the updation. Meanwhile, I am this mutation which basically uses a merge PR already created by GitHub (Yeah, I know it sounds unreal 😆), and applies it on top of the HEAD, this was custom made for PR branches.

@derberg
Copy link
Member

derberg commented Feb 21, 2024

the problem with saving minutes is that we have a huge amount of bot-created PRs, that quite often require autoupdating

one example: asyncapi/asyncapi-react#924 -> by default autoupdate is added and the bot merges master in like a charm

@Shurtu-gal
Copy link
Contributor Author

/help

@asyncapi-bot
Copy link

Hello, @Shurtu-gal! 👋🏼

    I'm 🧞🧞🧞 Genie 🧞🧞🧞 from the magic lamp. Looks like somebody needs a hand!

    At the moment the following comments are supported in pull requests:

    - `/ready-to-merge` or `/rtm` - This comment will trigger automerge of PR in case all required checks are green, approvals in place and do-not-merge label is not added
    - `/do-not-merge` or `/dnm` - This comment will block automerging even if all conditions are met and ready-to-merge label is added
    - `/autoupdate` or `/au` - This comment will add `autoupdate` label to the PR and keeps your PR up-to-date to the target branch's future changes. Unless there is a merge conflict or it is a draft PR.

@Shurtu-gal
Copy link
Contributor Author

/autoupdate

@Shurtu-gal
Copy link
Contributor Author

/update

@Shurtu-gal
Copy link
Contributor Author

@derberg
A successful run on comment:- https://github.com/ash17290/asyncapi-github/actions/runs/8086468906
A Successful run on push:- https://github.com/ash17290/asyncapi-github/actions/runs/8086440737

Note that only labelled PRs are auto-updated rest have to be manually done.

Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>
@Shurtu-gal Shurtu-gal requested a review from derberg March 11, 2024 16:58
@derberg
Copy link
Member

derberg commented Mar 12, 2024

@Shurtu-gal I see you requested review again, but I understand you will do further refactor first right?

@Shurtu-gal
Copy link
Contributor Author

@derberg The refactoring has been done.

.github/workflows/autoupdate-fork.yml Outdated Show resolved Hide resolved
.github/workflows/autoupdate-fork.yml Outdated Show resolved Hide resolved
.github/workflows/help-command.yml Outdated Show resolved Hide resolved
.github/workflows/autoupdate.yml Show resolved Hide resolved
@Shurtu-gal Shurtu-gal requested a review from derberg April 3, 2024 09:23
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

also in error messages for user, we do not write anything about merge conflicts, that in case of merge conflicts it will also not work

@Shurtu-gal
Copy link
Contributor Author

also in error messages for user, we do not write anything about merge conflicts, that in case of merge conflicts it will also not work

Added that as well.

@Shurtu-gal Shurtu-gal requested a review from derberg April 4, 2024 13:51
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

great stuff!!!

@derberg
Copy link
Member

derberg commented Apr 18, 2024

/rtm

@derberg derberg merged commit 9b309c0 into asyncapi:master Apr 18, 2024
7 checks passed
@Shurtu-gal Shurtu-gal deleted the feat/autoupdate-fork branch April 18, 2024 19:03
@Shurtu-gal
Copy link
Contributor Author

/help

@asyncapi-bot
Copy link

Hello, @Shurtu-gal! 👋🏼

    I'm 🧞🧞🧞 Genie 🧞🧞🧞 from the magic lamp. Looks like somebody needs a hand!

    At the moment the following comments are supported in pull requests:

    - `/please-take-a-look` or `/ptal` - This comment will add a comment to the PR asking for attention from the reviewrs who have not reviewed the PR yet.
    - `/ready-to-merge` or `/rtm` - This comment will trigger automerge of PR in case all required checks are green, approvals in place and do-not-merge label is not added
    - `/do-not-merge` or `/dnm` - This comment will block automerging even if all conditions are met and ready-to-merge label is added
    - `/autoupdate` or `/au` - This comment will add `autoupdate` label to the PR and keeps your PR up-to-date to the target branch's future changes. Unless there is a merge conflict or it is a draft PR. (Currently only works for upstream branches.)
    - `/update` or `/u` - This comment will update the PR with the latest changes from the target branch. Unless there is a merge conflict or it is a draft PR. NOTE: this only updates the PR once, so if you need to update again, you need to call the command again.

@Shurtu-gal
Copy link
Contributor Author

/update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Try to improve autoupdate
3 participants