-
Notifications
You must be signed in to change notification settings - Fork 107
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
Add support for allow_update_branch and update provider min version #148
base: main
Are you sure you want to change the base?
Conversation
78d3197
to
35515a0
Compare
Is there a planned timeline to merge this and also have the branch protection updated to fix the deprecation? Would like to have both in the module to provide this features to my colleagues. Thanks a lot! |
Hi @soerenmartius I don't know the module particularly well but thought I would try to provide some feedback. Thanks for updating. In general, I noticed that |
examples/public-repository/README.md
Outdated
@@ -62,7 +62,7 @@ module "repository" { | |||
|
|||
required_status_checks = { | |||
strict = true | |||
contexts = ["ci/travis"] | |||
checks = ["ci/travis"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that this uses branch_protection
and not branch_protection_v3
so therefore contexts
is still the correct term at least that is how I understand the documentation.
examples/public-repository/main.tf
Outdated
@@ -47,7 +48,7 @@ module "repository" { | |||
|
|||
admin_collaborators = ["terraform-test-user-1"] | |||
|
|||
branch_protections = [ | |||
branch_protections_v5 = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not able to find a variable known as branch_protections_v5
in the code. I can find branch_protections_v4
and was wondering if this line is supposed to reference that variable instead? If so, I think we should rename all references to either branch_protections_v5
or just branch_protections
.
@soerenmartius, want to look into updating the PR with the comments made from @switchdk? |
Since this is going to be a breaking change, can we also please obsolete the I could create a pull request for it targeting this branch if that is okay with the maintainers. |
35515a0
to
47f6413
Compare
…checks.checks as contexts is depcrecated
47f6413
to
f39f6e7
Compare
yeah that's a fair point - I wanted to do this since ages but as we're currently focussing on https://github.com/mineiros-io/terramate I really didn't find the time yet |
Let me discuss with the team if we can invest some time in a major refactoring |
Hello dear @soerenmartius , any possible date to fix this issue? |
@soerenmartius hi, is the required refactoring something that could be written down, then maybe somebody could contribute it :) |
allow_update_branch
. Bump minimum supported version of the GitHub provider tov5.16
as it contains a critical fix for branch protections.