Skip to content

fix: Include only required checks while evaluating the mergeable state when apply is ignored#1

Open
bhavith wants to merge 1 commit intofinnag:cogfrom
bhavith:fix-mergeable-checks
Open

fix: Include only required checks while evaluating the mergeable state when apply is ignored#1
bhavith wants to merge 1 commit intofinnag:cogfrom
bhavith:fix-mergeable-checks

Conversation

@bhavith
Copy link

@bhavith bhavith commented Oct 27, 2023

what

Currently when the apply condition is set to mergeable along with --gh-allow-mergeable-bypass-apply flag, Atlantis considers non required status checks as well to decide whether the PR is mergeable or not (to allow apply or not). This is a deviation from standard github mergeable status.

This PR changes that and includes only required status checks while evaluating mergeable status.

why

Atlantis uses mergeable status from github API to check if a PR is mergeable if --gh-allow-mergeable-bypass-apply is not set. That is if the status is unstable, the PR is considered mergeable.

unstable: Failing/pending commit status that is not part of the required
	//           status checks. Merging is allowed (yellow box).

If --gh-allow-mergeable-bypass-apply enabled, github uses some additional checks to see if the PR is mergeable without considering atlantis apply. But in these additional checks, Atlantis looks at all statuses irrespective of if they are required or not. So, if there is a status check that is failing Atlantis will consider it non mergeable even if its not a required status check and thus will block Apply.

In this change, while checking if the state is success, it will also check if the particular status is one of the required checks. Hence Atlantis will report non mergeable only if the failing status check is a required one.

tests

  • I have adjusted the test data to include scenarios where there could be failing non required checks
  • I have tested this by running atlantis locally against a terraform repository with a few valid scenarios

@bhavith bhavith changed the title Include required checks while evaluating the mergeable state when apply is ignored fix : Include required checks while evaluating the mergeable state when apply is ignored Oct 27, 2023
@bhavith bhavith changed the title fix : Include required checks while evaluating the mergeable state when apply is ignored fix: Include required checks while evaluating the mergeable state when apply is ignored Oct 27, 2023
@bhavith bhavith changed the title fix: Include required checks while evaluating the mergeable state when apply is ignored fix: Include only required checks while evaluating the mergeable state when apply is ignored Nov 3, 2023
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.

1 participant