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

fix: allow Require Linear History when selecting merge method #3211

Merged

Conversation

tpolekhin
Copy link
Contributor

@tpolekhin tpolekhin commented Mar 15, 2023

what

Account for branch protection rule "Require Linear History" when choosing merge method

why

When repo config allows merge commits:
Screenshot 2023-03-18 at 13 40 13

But branch protection rules requires linear history (aka banning merge commits):
Screenshot 2023-03-18 at 13 40 36

Current logic will always pick merge method by default. This PR fixes that.

tests

  • I have updated TestGithubClient_MergePullCorrectMethod test
  • I have tested my changes by running tests from VSCode
  • I have tested my changes by running make test
  • I have tested my changes on test environment

references

@tpolekhin tpolekhin requested a review from a team as a code owner March 15, 2023 15:10
@github-actions github-actions bot added go Pull requests that update Go code provider/github labels Mar 15, 2023
@tpolekhin tpolekhin force-pushed the fix/github-merge-require-linear-history branch from 93e2d02 to d62d733 Compare March 15, 2023 21:23
@tpolekhin
Copy link
Contributor Author

@nitrocode I have fixed the logic a little bit and also added test. deploy/netlify fails because of some frontend issue I think after the recent renovate deps update - I didn't touch the frontend part in my PR.
Could you please approve test workflows here, or you want to wait until the frontend issue is fixed and then rebase this PR?

@nitrocode
Copy link
Member

Please ignore the netlify error and please fix the tester error.

@tpolekhin
Copy link
Contributor Author

Please ignore the netlify error and please fix the tester error.

@nitrocode do you mean tester / website_link_check? I think this one is also affected by the frontend issue

@nitrocode
Copy link
Member

Yes please ignore the netlify error. Seems to have come from this pr #3225

@nitrocode
Copy link
Member

Please fill out the pr template

@tpolekhin
Copy link
Contributor Author

tpolekhin commented Mar 18, 2023

@nitrocode I have tested this change on the actual GitHub repository
https://github.com/tpolekhin/atlantis-test-terraform

  • main branch - unprotected - automerge successful after atlantis apply - PR2
  • protected branch - protected without linear history - successful - PR3
  • protected-linear branch - protected WITH linear history - successful - PR4

Atlantis logs: https://gist.github.com/tpolekhin/eb63628798b82e7584d3f2eac14dcd16

@nitrocode nitrocode changed the title fix: account for Require Linear History when selecting merge method fix: allow Require Linear History when selecting merge method Mar 18, 2023
@nitrocode nitrocode merged commit 7a33828 into runatlantis:main Mar 18, 2023
10 checks passed
@nitrocode
Copy link
Member

@tpolekhin great work and testing! Please feel free to propose other PRs if you have the time. And please join the #contributors channel in the atlantis slack community. Thank you!

@nitrocode nitrocode added this to the v0.23.3 milestone Mar 18, 2023
@tpolekhin
Copy link
Contributor Author

@nitrocode thank you for the great feedback and for the merge!

GenPage added a commit that referenced this pull request Apr 11, 2023
This reverts commit ebc06c1.

Revert "fix: allow `Require Linear History` when selecting merge method (#3211)"

This reverts commit 7a33828.
GenPage added a commit that referenced this pull request Apr 11, 2023
Reverts "fix(github): branch protection not supported (#3276)"
Reverts "fix: allow Require Linear History when selecting merge method (#3211)"
Closes #3320
@nitrocode
Copy link
Member

@tpolekhin this was reverted due to a breaking change. See the related pr #3321

nitrocode added a commit that referenced this pull request May 5, 2023
* fix: account for Require Linear History when selecting merge method

* fix logic and add tests

* add nolint hints for w.Write

---------

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
nitrocode pushed a commit that referenced this pull request May 5, 2023
Reverts "fix(github): branch protection not supported (#3276)"
Reverts "fix: allow Require Linear History when selecting merge method (#3211)"
Closes #3320
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
…tlantis#3211)

* fix: account for Require Linear History when selecting merge method

* fix logic and add tests

* add nolint hints for w.Write

---------

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
Reverts "fix(github): branch protection not supported (runatlantis#3276)"
Reverts "fix: allow Require Linear History when selecting merge method (runatlantis#3211)"
Closes runatlantis#3320
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
…tlantis#3211)

* fix: account for Require Linear History when selecting merge method

* fix logic and add tests

* add nolint hints for w.Write

---------

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
Reverts "fix(github): branch protection not supported (runatlantis#3276)"
Reverts "fix: allow Require Linear History when selecting merge method (runatlantis#3211)"
Closes runatlantis#3320
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code provider/github
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automerge support for "require linear history"
2 participants