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

command/jsonprovider: bump format version #28115

Merged
merged 1 commit into from
Mar 22, 2021
Merged

Conversation

mildwonkey
Copy link
Contributor

Support for attributes with NestedTypes was added in #28055, and that PR should have included a format version bump: this is a backwards-compatible change, but consumers will need to be updated in order to properly decode attributes (with NestedTypes) going forward.

Thanks @bflad for the nudge!

@vancluever , I am sorry I did not include you on the last PR (#28055); if you have questions / see any problems / have any requests I'll get right on 'em.

Support for attributes with NestedTypes was added in #28055, and should have included a format version bump: this is a backwards-compatible change, but consumers will need to be updated in order to properly decode attributes (with NestedTypes) going forward.
@mildwonkey mildwonkey requested review from vancluever and a team March 16, 2021 20:07
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

LGTM, I already have a draft PR, so I'll just reflect this version change there.

hashicorp/terraform-json#28

I reckon instead of string-matching the version we'll need to start parsing it as semver and check if it's =< than supported (0.2) and then continue erroring in the unmarshaller if it's not. Alternatively we can continue exact-matching it and have a slice of two strings. 🤷🏻‍♂️

Copy link
Contributor

@vancluever vancluever left a comment

Choose a reason for hiding this comment

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

Seeing as this is just a version bump, 👍 here too.

We'll look at the implications of this over in terraform-json.

Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Thank you so much!

@mildwonkey mildwonkey added the 0.15-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Mar 22, 2021
@mildwonkey mildwonkey merged commit 77562d9 into main Mar 22, 2021
@mildwonkey mildwonkey deleted the mildwonkey/json-format-v branch March 22, 2021 15:45
@ghost
Copy link

ghost commented Apr 22, 2021

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
0.15-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants