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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

wait-for-checks - Rewrite for reliability #7073

Closed
wants to merge 3 commits into from
Closed

Conversation

fregante
Copy link
Member

@fregante fregante commented Nov 16, 2023

I hate this feature, it's important and it's complex. If I don't fix it this time I'll drop it.

  • UI changes
    • checkbox must be updated depending on
      • the status of CI
      • visibility of merge box
  • Merging
    • merge box must be dimmed when user presses "merge" and
      • checkbox is present and checked
    • merge box must be dimmed until
      • new commit comes in
      • CI passes
      • CI fails
      • user presses cancel
    • merging can proceed when
      • CI passes

I think this can be simplified a bit by matching the behavior or GitHub's auto-merge, which doesn't care about new commits or failing CI. It will just merge the PR when checks pass.

  • Edit: Done 馃殌

Without high-level helpers from framework, this produces a bit of spaghetti.

Test URLs

refined-github/sandbox#12

Screenshot

Still a WIP

Related

The last rewrite was just shy of 2 years ago.

@fregante fregante added the bug label Nov 16, 2023
@FloEdelmann FloEdelmann deleted the wait-for-checks-anew branch February 5, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

wait-for-checks is missing or it does not merge the PR
1 participant