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

tree-wide: fix references to the terraform-providers organization #67

Closed
wants to merge 1 commit into from

Conversation

flokli
Copy link

@flokli flokli commented Apr 2, 2020

terraform-provider-archive has been moved to the hashicorp
organization.

`terraform-provider-archive` has been moved to the `hashicorp`
organization.
@flokli flokli mentioned this pull request Apr 2, 2020
10 tasks
@flokli
Copy link
Author

flokli commented Jun 3, 2020

ping ;-)

@kmoe
Copy link
Member

kmoe commented Jun 26, 2020

Thank you for the PR.

We do not support importing Terraform providers as Go modules, so it is not necessary to update the module path for this provider. Please see our best practice documentation.

While we appreciate the effort that goes into preparing PRs, there is always a tradeoff between benefit and cost. In this case, the proposed change would break in-flight pull requests for this provider, creating extra vendor diffs and extra work for our contributors. We therefore will not merge this PR at this time.

If the module path is causing issues with something besides importing the provider, we'd love to hear more about it. Reply to this issue and let us know what you're trying to do, and we'll look into it.

@kmoe kmoe closed this Jun 26, 2020
@flokli
Copy link
Author

flokli commented Jul 6, 2020

Thanks for getting back to me on this.

If the module path is causing issues with something besides importing the provider, we'd love to hear more about it. Reply to this issue and let us know what you're trying to do, and we'll look into it.

I don't import Terraform providers as a go module in my code, but the build tooling and packaging code around fails to build with the wrong names. Please see the referencing issue NixOS/nixpkgs#83279 for details and the other issues linked in #67 (comment) for the fallout currently required to get them to build.

I don't really see how this can largely affect other in-flight pull requests - it shouldn't change any vendor diffs in this repo, and this is mostly to be consistent with where the code now lives.

@flokli
Copy link
Author

flokli commented Jul 9, 2020

Also note the current approach seems to be named the "module name workaround", and fixing the references, as proposed here and in the other 8 PRs seems to be preferred, as per hashicorp/terraform-provider-helm#522:

Instead of using the module name workaround from step 7 of the migration RFC
(https://docs.google.com/document/d/1PQFAhpTgUhHcZxIIWapNsa0UTPOeC5y8DO0zuV1mqCk/edit?ts=5ed8d6e0)
it's more straightforward to just rename the module.

The PRs did all the heavy lifting, the number of other in-flight PRs affected by this is pretty minimal, and sticking with the workaround is causing problems currently just pushed to downstream packagers.

Please reconsider, also cc @dak1n1 @aareet .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants