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

Update global address docs now that they are settable + ip ranges sometimes #1617

Merged

Conversation

rileykarson
Copy link
Member

What a weird situation for MM! The resource has changed a fair bit @ beta vs GA, so
we need separate docs for Ansible/InSpec vs Terraform.

This isn't super explicit about the behaviour of google here, where I think it can still only be a single address. Do you think we should go fire and brimstone, or that it will be a non-issue?


[all]

[terraform]

[terraform-beta]

[ansible]

[inspec]

@rileykarson rileykarson requested a review from chrisst April 3, 2019 22:46
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
This PR seems not to have generated downstream PRs before, as of 5f924fd.

Pull request statuses

No diff detected in terraform-google-conversion.

New Pull Requests

I built this PR into one or more new PRs on other repositories, and when those are closed, this PR will also be merged and closed.
depends: hashicorp/terraform-provider-google-beta#585
depends: hashicorp/terraform-provider-google#3376
depends: modular-magician/ansible#228
depends: modular-magician/inspec-gcp#146

Copy link
Contributor

@chrisst chrisst left a comment

Choose a reason for hiding this comment

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

I think this approach is good for now, especially if it's just docs that need changing. I might have chosen to invert the description override (add beta description overrides to Terraform) but it's pretty slim difference.

@rileykarson
Copy link
Member Author

I debated between the two; I think my model is that api.yaml is the canonical representation of the API as a whole across versions and so it should represent the most complete model of information. In that frame, InSpec and Ansible not supporting beta is a provider-specific detail and so they use an override for the description to match what they support.

Does that model make sense to you? We can break this into an MM sync to gather feedback across the team if you think it's contentious.

Also, if versioned descriptions become a problem we can investigate adding different values for different versions. In the past, we've seen required fields become optional & more.

@chrisst
Copy link
Contributor

chrisst commented Apr 4, 2019

I had approached it from the perspective of "terraform is the only provider that supports beta, therefore it should contain the override". It would also mean less duplication of the overrides. But I don't think it's worth syncing up about. Both are compelling to me and I don't see significant downsides of arbitrarily picking one.

Tracked submodules are build/terraform-beta build/terraform-mapper build/terraform build/ansible build/inspec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants