Fix issues with auto_init destroying repositories#317
Merged
jcudit merged 3 commits intointegrations:masterfrom Jun 9, 2020
Merged
Conversation
d9fa07e to
ec1d732
Compare
Contributor
|
@majormoses would you mind resolving the merge conflicts? |
Contributor
Author
Sure, I have not bothered keeping it up to date since it was not getting any love from maintainers. |
ec1d732 to
cdfb104
Compare
From github API perspective setting or modifying `auto_init` only makes sense in the context of creating a new repository. Changing it after is ignored by the github API which is the behavior we should (and use to) match. The only scenario where this makes sense to assume that changing this intends on a destructive action (blowing up a github repo is not something that should be easy accidentally) is when you use the `terraform taint` command but for very different reasons. This change is related to several PRs that subverts the communities expectations with no real or perceived value. Signed-off-by: Ben Abrams <me@benabrams.it>
cdfb104 to
8e9ea3c
Compare
Contributor
Author
|
I think I got the tests working again but it looks like there are some unrelated errors: https://travis-ci.org/github/terraform-providers/terraform-provider-github/jobs/678001033#L497 |
anGie44
reviewed
May 25, 2020
Collaborator
anGie44
left a comment
There was a problem hiding this comment.
thanks for this PR @majormoses! just one suggestion but otherwise LGTM 👍
Output of acceptance tests:
--- PASS: TestAccGithubRepository_archive (8.14s)
--- PASS: TestAccGithubRepository_createFromTemplate (8.82s)
--- PASS: TestAccGithubRepository_templates (9.72s)
--- PASS: TestAccGithubRepository_hasProjects (11.15s)
--- PASS: TestAccGithubRepository_archiveUpdate (13.19s)
--- PASS: TestAccGithubRepository_basic (14.19s)
--- PASS: TestAccGithubRepository_defaultBranch (16.06s)
--- PASS: TestAccGithubRepository_topics (19.63s)
majormoses
commented
May 25, 2020
jcudit
approved these changes
Jun 9, 2020
Contributor
|
Thanks again for fixing this one @majormoses. Glad this annoyance has been removed. |
Contributor
Author
|
Happy to help, looking forward to ripping some hacks out of our modules 😀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
From github API perspective setting or modifying
auto_initonly makes sense in the context of creating a new repository. Changing it after is ignored by the github API which is the behavior we should (and use to) match. The only scenario where this makes sense to assume that changing this intends on a destructive action (blowing up a github repo is not something that should be easy accidentally) is when you use theterraform taintcommand but for very different reasons. This change is related to several PRs and issues that subverts the communities expectations with no real or perceived value.For further context please see #155 #154 https://github.com/sous-chefs/terraform-github-org/pull/111 #135
fixes #155
Signed-off-by: Ben Abrams me@benabrams.it