This repository has been archived by the owner on Dec 24, 2023. It is now read-only.
Settle on concrete dependencies using Dep #2
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
This commit also accomodates two breaking changes in the Google Go SDK: one, where a type was renamed, and two, when an API parameter was generalized.
5 tasks
|
You, my friend, are a rock star 🎸. LGTM! |
lukehoban
reviewed
Jun 5, 2018
main.go
Outdated
| @@ -2,7 +2,7 @@ package main | |||
|
|
|||
| import ( | |||
| "github.com/hashicorp/terraform/plugin" | |||
| "github.com/terraform-providers/terraform-provider-google/google" | |||
| "github.com/pulumi/terraform-provider-google/google" | |||
There was a problem hiding this comment.
Ideally we would leave this with the same package identity, and just bind to our fork via a dep repository mapping. Any reason you needed to change this?
There was a problem hiding this comment.
Dep got extremely confused by this import and kept trying to make this repo vendor itself. I'm not sure why this happened but I can look into it in more detail if this is a little scary.
There was a problem hiding this comment.
Figured this out, I did something dumb with GOPATH.
|
Going to go ahead and merge this to get GCP rolling - I fixed the problem Luke pointed out and we should be good to go. Let me know if I should revert this. |
|
Great - thanks! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This commit also accomodates two breaking changes in the Google Go SDK:
one, where a type was renamed, and two, when an API parameter was
generalized.
This repo originally used
govendor, which is willing to vendor individual subpackages of one repo at different commit hashes. Conceptually, this is not a problem for Go and there's no reason why this is invalid, but the reality is thatdepis very opinionated and does not allow this. Dep requires that all packages within a repo come from the same commit hash.This poses a problem for us since the upstream repo vendors subpackages of
cloud.google.com/goat different commit hashes. Sincedepis absolutely not willing to budge on this issue, our options are 1) accept that we must usegovendorand integrategovendorinto the GCP provider build or 2) force this repo to adhere todep, which means locking to a specific version ofcloud.google.com/goand addressing the breakage that arises from that.This PR takes option number 2 because there are only two places where locking
cloud.google.com/gocaused breakage:cloudbilling.Servicetocloudbilling.APIServicewithout bumping semverterraform-provider-googleuses Beta Compute APIs to implement thegoogle_compute_subnetwork_iam_policy, which made a breaking change.I tried to run the upstream acceptance tests that test
google_compute_subnetwork_iam_policy, but I couldn't get them to pass in the pristine upstream repo. The tests fail in the exact same way both with and without our fork's code.