Skip to content

Don't apply wait-for-build when auto-merge is enabled #3893

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
Merged
fregante merged 3 commits into from
Jan 17, 2021
Merged

Don't apply wait-for-build when auto-merge is enabled #3893

fregante merged 3 commits into from
Jan 17, 2021

Conversation

@Jackenmen
Copy link
Contributor

@Jackenmen Jackenmen commented Jan 13, 2021 •
edited
Loading

  1. LINKED ISSUES:
    Fixes Disable wait-for-build when Github auto-merge is enabled #3887

  2. TEST URLS:
    Not possible to provide, the tester will need to make a repo with proper conditions met themselves.
    See SCREENSHOT section below for possible conditions.

  3. SCREENSHOT:

    • Auto-merge (GH only shows it when there are pending required checks):
      image
    • Regular merge, auto-merge enabled, pending optional checks:
      image
    • Regular merge, auto-merge enabled, all optional and required checks have passed already:
      image
    • Admin merge (GH only shows it when there are pending required checks), auto-merge enabled:
      image
      image
    • Regular merge, auto-merge disabled, pending optional checks:
      image
      image
    • Admin merge (GH only shows it when there are pending required checks), auto-merge disabled:
      image
      image

@Jackenmen Jackenmen marked this pull request as ready for review January 13, 2021 09:47
@Jackenmen
Copy link
Contributor Author

Jackenmen commented Jan 13, 2021

Should be ready for review now.

fregante
fregante reviewed Jan 13, 2021
View reviewed changes
@fregante fregante added the bug label Jan 13, 2021
@fregante
Copy link
Member

fregante commented Jan 13, 2021

Thanks for the PR and the many screenshots!

Jackenmen and others added 2 commits January 14, 2021 08:45
@Jackenmen @fregante
Selector magic :)
be0b99b 
Co-authored-by: Federico <opensource@bfred.it>
@Jackenmen
Fix syntax error
0f662a1
fregante
fregante approved these changes Jan 14, 2021
View reviewed changes
Copy link
Member

@fregante fregante left a comment

Choose a reason for hiding this comment

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

The behavior is unchanged for non-auto-merge users, but I assume that you tested the latest version and that it works for you for auto-merge PRs

@Jackenmen
Copy link
Contributor Author

Jackenmen commented Jan 14, 2021

I have tested the first version, but I have not tested the version with changes after your review comment, but I don't have much reason to believe it changes anything as it really just simplified the code that was there before without changing what it actually does.

@fregante
Copy link
Member

fregante commented Jan 16, 2021 •
edited
Loading

I have not tested the version with changes after your review comment

It might be worth checking it out since that's the only change this PR is expected to make 😄 or else we're back to square 1 tomorrow. Maybe the loop was necessary after all, but we can't know until someone tests it.

@Jackenmen
Copy link
Contributor Author

Jackenmen commented Jan 17, 2021

Alright, just checked all the conditions from the PR description and they still work. Not gonna post screenshots again since it all looks the same as before.

@fregante fregante changed the title Disable wait-for-build for auto merge form Don't apply wait-for-build when auto-merge is enabled Jan 17, 2021
@fregante fregante merged commit f292d45 into refined-github:master Jan 17, 2021
@fregante
Copy link
Member

fregante commented Jan 17, 2021

Perfect, thanks!

@Jackenmen Jackenmen deleted the patch-1 branch January 17, 2021 10:31
@tom-sherman
Copy link
Contributor

tom-sherman commented Jan 17, 2021

Amazing work, thanks so much!

@vbouchet31 vbouchet31 mentioned this pull request Jan 19, 2021
Closed
@fregante fregante mentioned this pull request Feb 4, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fregante fregante fregante approved these changes

Assignees

No one assigned

Labels

bug

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Disable wait-for-build when Github auto-merge is enabled

3 participants

@Jackenmen @fregante @tom-sherman