Fix issues with auto_init destroying repositories #317
Conversation
This file contains 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
ghost
added
the
majormoses
force-pushed
the
fix/155-auto_init-should-not-be-ForceNew
branch
from
d9fa07e
to
ec1d732
Compare
|
@majormoses would you mind resolving the merge conflicts? |
Sure, I have not bothered keeping it up to date since it was not getting any love from maintainers. |
majormoses
force-pushed
the
fix/155-auto_init-should-not-be-ForceNew
branch
from
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>
majormoses
force-pushed
the
fix/155-auto_init-should-not-be-ForceNew
branch
from
cdfb104
to
8e9ea3c
Compare
|
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
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
|
Thanks again for fixing this one @majormoses. Glad this annoyance has been removed. |
|
Happy to help, looking forward to ripping some hacks out of our modules 😀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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