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

Added inputs, deprecated source_json #14

Merged
merged 7 commits into from
Jul 13, 2022
Merged

Conversation

nitrocode
Copy link
Member

@nitrocode nitrocode commented Jul 9, 2022

@nitrocode nitrocode requested review from a team as code owners July 9, 2022 13:30
@nitrocode nitrocode requested review from adamcrews and milldr July 9, 2022 13:30
@nitrocode
Copy link
Member Author

/test all

@nitrocode
Copy link
Member Author

/test all

@nitrocode
Copy link
Member Author

/test all

@nitrocode nitrocode requested review from aknysh and Gowiem July 11, 2022 15:08
Copy link
Member

@Gowiem Gowiem left a comment

Choose a reason for hiding this comment

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

@nitrocode looks like a great upgrade 👍 One nitpick on the description of the updated var, but otherwise I think it looks ready to go.

examples/complete/variables.tf Outdated Show resolved Hide resolved
examples/complete/versions.tf Show resolved Hide resolved
main.tf Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
@Gowiem
Copy link
Member

Gowiem commented Jul 11, 2022

@nitrocode I imagine this should be a major version rev, correct? Do we have automation in place to make that happen or do you need to do this as a non-release and then release manually?

@nitrocode
Copy link
Member Author

/test all

@nitrocode
Copy link
Member Author

@nitrocode I imagine this should be a major version rev, correct? Do we have automation in place to make that happen or do you need to do this as a non-release and then release manually?

Hmm I don't know. The patch version is for backwards compatibility. The minor version is for adding new functionality in a backwards compatible manner. And major version is for breaking changes. I suppose this is a breaking change unless I add a variables-deprecated.tf file and if source_json input is used, it uses a local to combine it with the source_policy_documents. I'd prefer to not release a major version for now since this module is still kind of "incubating" since it's new.

Perhaps I should go with the deprecated route to avoid breaking changes then?

@Gowiem
Copy link
Member

Gowiem commented Jul 12, 2022

@nitrocode Eh, if we don't want to major rev because we're still considering it incubating then that seems like a good enough reason to me to just rev minor and skip the deprecated workaround. Don't want to create a big hoop to jump through just because we're trying to move towards reving major versions... we haven't done it everywhere yet AFAICT so it seems like adding more work when it's likely overkill.

Thanks for jumping on my nitpick 👍

@Gowiem
Copy link
Member

Gowiem commented Jul 12, 2022

@nitrocode leaving up to you to merge 👍

@nitrocode nitrocode merged commit 374875a into master Jul 13, 2022
@nitrocode nitrocode deleted the source_policy_documents branch July 13, 2022 19:13
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.

source_json is deprecated. Use the attribute "source_policy_documents" instead.
3 participants