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 issues with auto_init destroying repositories #317

Conversation

majormoses
Copy link
Contributor

@majormoses majormoses commented Jan 5, 2020

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 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 [email protected]

@ghost ghost added the size/S label Jan 5, 2020
@majormoses majormoses force-pushed the fix/155-auto_init-should-not-be-ForceNew branch from d9fa07e to ec1d732 Compare January 5, 2020 22:30
@ghost ghost added size/M and removed size/S labels Jan 5, 2020
@yordis
Copy link

yordis commented Mar 5, 2020

@majormoses would you mind resolving the merge conflicts?

@majormoses
Copy link
Contributor Author

@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 majormoses force-pushed the fix/155-auto_init-should-not-be-ForceNew branch from ec1d732 to cdfb104 Compare April 21, 2020 18:30
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 <[email protected]>
@majormoses majormoses force-pushed the fix/155-auto_init-should-not-be-ForceNew branch from cdfb104 to 8e9ea3c Compare April 22, 2020 03:49
@majormoses
Copy link
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

Copy link
Contributor

@anGie44 anGie44 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

github/resource_github_repository.go Outdated Show resolved Hide resolved
@jcudit jcudit added this to the v2.8.1 milestone Jun 3, 2020
@jcudit jcudit merged commit 399b54a into integrations:master Jun 9, 2020
@jcudit
Copy link
Contributor

jcudit commented Jun 9, 2020

Thanks again for fixing this one @majormoses. Glad this annoyance has been removed.

@majormoses
Copy link
Contributor Author

Happy to help, looking forward to ripping some hacks out of our modules 😀

@majormoses majormoses deleted the fix/155-auto_init-should-not-be-ForceNew branch June 9, 2020 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changing auto init value should not force a repo to be destroy/created
4 participants