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

refactor!: Improve Mappings helper function(s) #387

Merged
merged 1 commit into from
Apr 7, 2021

Conversation

jacobwinch
Copy link
Contributor

What does this change?

The setStageDependentValue and getStageDependentValue helper functions have been removed. In order to reduce boilerplate, the same functionality is now provided via a single function (withStageDependentValue).

Does this change require changes to existing projects or CDK CLI?

Yes, this is a breaking change. Stacks which use setStageDependentValue and getStageDependentValue directly will need to use withStageDependentValue instead. Stacks which use this functionality via the ASG construct should be unaffected.

How to test

Existing ASG unit tests already validate this behaviour (and they still pass!).

How can we measure success?

  • Less boilerplate is needed to use stage-dependent values.

Have we considered potential risks?

As mentioned above, this is a breaking change. However, as this functionality is not widely adopted yet I think it's relatively low risk to make the change at this early stage of development.

BREAKING CHANGE: The `setStageDependentValue` and `getStageDependentValue` helper functions have been removed. In order to reduce boilerplate, the same functionality is now provided via a single function (`withStageDependentValue`).
@jacobwinch jacobwinch marked this pull request as ready for review April 6, 2021 14:55
@jacobwinch jacobwinch requested a review from a team April 6, 2021 14:55
@jacobwinch
Copy link
Contributor Author

Stacks which use setStageDependentValue and getStageDependentValue directly will need to use withStageDependentValue instead.

cc @mbarton as I think you might be the only affected user so far! It would be good to know if you think that this is an improvement or not.

Copy link

@mbarton mbarton left a comment

Choose a reason for hiding this comment

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

👍 this makes it much less likely for people to try and do logic on it or assign the result to a variable

@jacobwinch jacobwinch merged commit e001123 into main Apr 7, 2021
@jacobwinch jacobwinch deleted the jw-improve-stage-dependent-helpers branch April 7, 2021 06:24
@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2021

🎉 This PR is included in version 7.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants