Skip to content

Skip CI if merge conflicts exist #12627

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

crertel
Copy link

@crertel crertel commented Mar 10, 2025

Motivation

This PR stops CI from running at all if merge conflicts exist.

Context

I noticed that another PR kicked off a bunch of CI despite being full of merge conflicts--this seems like a waste of resources.

I'm not sure this is the best way to fix that, or even if we want to skip CI when merge conflicts exist, but I figured this is a starting point.


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@crertel crertel requested a review from edolstra as a code owner March 10, 2025 19:11
@crertel crertel changed the title Skip CI if merge conflict exists. Skip CI if merge conflicts exist Mar 10, 2025
@Jaculabilis
Copy link
Contributor

GitHub Actions triggered by on.pull_request are already skipped if there are merge conflicts. See this thread for a lot of people being caught off guard by this, with the linked comment providing the technical explanation:

Apparently, pull_request event workflows run on a temporary merge commit. Therefore, whenever there are merge conflicts, the temporary merge commit will automatically abort, as there's no way to solve the conflicts automatically—this prevents the pull_request event workflow from triggering.

The ci.yaml action has a on.push trigger, which is presumably responsible for the triggers on PRs with merge conflicts.

I'm not a SME on the place of ci.yaml in the operation of nixpkgs but the issue might be solved simply by removing or filtering the on.push trigger. The documentation is not very clear about what the different activity types mean, but I think the default of synchronize, opened, and reopened generally lines up with the expected behavior of "run when the merge HEAD changes"? At $WORK we use [synchronize, opened, reopened, edited] but I think that last one just triggers when the PR description is edited.

Note also that the trigger setup dates to the initial commit and hasn't been touched since. So removing on.push or filtering it to non-feature branches is probably the right move here rather than expanding the amount of YAML.

@crertel
Copy link
Author

crertel commented Mar 10, 2025

@Jaculabilis I think the intent (though I may be wrong) is that CI still runs as commits are pushed, so I don't know if removing thepush trigger is the play. That said, it'd be a hell of a lot simpler than where I started if we could do it, I think! Good find. :)

@Jaculabilis
Copy link
Contributor

Yes, if we want to run CI when e.g. commits are pushed directly to master, then we need on.push. That would also be necessary to run CI on the merge/squash/rebase commit produced by a pull request merging. But then we should be able to filter it to just the long-lived release branches and not feature branches.

console.log("Not a pull request, skipping merge conflict check.");
return;
}
const { data: pr } = await github.rest.pulls.get({
Copy link
Member

@Mic92 Mic92 Mar 11, 2025

Choose a reason for hiding this comment

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

I don't know we can use this endpoint. I know from experience that github takes a while to figure out if something can be merged or not. Isn't the error message from the checkout action enough? This would run another action to the average pull request, while merge conflicts in pull requests are rather the exception.

Copy link
Author

Choose a reason for hiding this comment

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

I'm totally down to find another way of accomplishing this--you hit on the head my biggest concern about Github taking too long to settle (we don't want to have maintainers constantly having to kick CI). Can you give a bit more color to what you're thinking with the checkout action?

As for the rest, I don't really think having an extra action (especially if it prevents other bogus runs) is a big deal, and I'd hope that if development pace picks up we'd see more potential conflicts.

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

Successfully merging this pull request may close these issues.

3 participants