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

PR gets merged before all workflows were successful #1212

Open
DanielBadura opened this issue Dec 2, 2021 · 4 comments
Open

PR gets merged before all workflows were successful #1212

DanielBadura opened this issue Dec 2, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@DanielBadura
Copy link

DanielBadura commented Dec 2, 2021

How would you describe the issue?

First, im not sure if this is a real bug here or something wrong with the config or gh workflow fail, maybe you can help me?

I recently added the workflow for auto-merges here: https://github.com/maglnet/ComposerRequireChecker/blob/3.6.x/.github/workflows/merge-dependabot-upgrades.yml
I wanted that these workflows listed there should all be successfully run and then the PR from dependatbot should be auto-merged.

Here one PR that was merged to fast: maglnet/ComposerRequireChecker#323

What are the expected results?

I would expect that all jobs need to be successful for the auto-merge.

What are the actual results?

The merge happens before all job were green.

How much does it hurt?

The solution would be to put all workflow files into one "Continues Integration" which is really not nice for maintainance. For example here: https://github.com/Roave/BetterReflection/tree/5.0.x/.github/workflows

@DanielBadura DanielBadura added the bug Something isn't working label Dec 2, 2021
@rbubley
Copy link

rbubley commented Apr 21, 2022

This was discussed (but no solution) here: https://stackoverflow.com/questions/67721399/run-a-workflow-when-multiple-other-workflows-have-finished-in-github-actions.

This looks like a promising approach, but have not tested: https://github.com/marketplace/actions/wait-on-check

@dunyakirkali
Copy link
Contributor

Isn't it because dependabot has been added as an admin, and the repo is set to not include administrators in the merge check that it gets merged?

or is this something else?

@aaneitchik
Copy link
Contributor

@dunyakirkali I tried looking into this, and I believe @rbubley is right: the problem here is that in the example that @DanielBadura shared

on:
  workflow_run:
    types:
      - completed
    workflows:
      - "Check Coding Standards"
      - "Static Analysis by PHPStan"
      - "Static Analysis by Psalm"
      - "Static Analysis by Require-Checker"
      - "PHPUnit tests"

there are multiple workflows, and looking at the GitHub docs, in particular

If you specify multiple workflows for the workflow_run event, only one of the workflows needs to run. For example, a workflow with the following trigger will run whenever the "Staging" workflow or the "Lab" workflow completes.

on:
workflow_run:
  workflows: [Staging, Lab]
 types:
   - completed

so as soon as one of the workflow finishes, this will be true: github.event.workflow_run.conclusion == 'success' and our action will run. I may be missing something, but unfortunately I'm afraid going with a single workflow is the only way to make the action work

@DanielBadura
Copy link
Author

Maybe this should then be documented here since it can be misleading right now.

plata added a commit to PhoenicisOrg/phoenicis that referenced this issue Nov 20, 2022
As described in ridedott/merge-me-action#1212, the action runs as soon as any workflow in workflows completed. This means that it usually ran after linting the Java files and thus ignored if the CI failed.

Ignoring the linting workflow is not optimal. However, the current behavior is way worse. Also, merge-me is meant for dependabot PRs which should not produce linting issues in general.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants