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

allow $merge entries in cfn Resources: #24

Merged
merged 1 commit into from
Dec 10, 2017

Conversation

tavisrudd
Copy link
Collaborator

The commit also refactors the implementation of visitResourceNode and
moves custom resource handling to visitCustomResource.

@tavisrudd tavisrudd changed the title allow $merge entries in cnf Resources: allow $merge entries in cfn Resources: Dec 10, 2017
@tavisrudd
Copy link
Collaborator Author

oops. type in the commit message. fixing ...

The commit also refactors the implementation of `visitResourceNode` and
moves custom resource handling to `visitCustomResource`.
@tavisrudd tavisrudd force-pushed the allow-$merge-in-cfn-Resources-node branch from 2017e1a to 1d1fd1c Compare December 10, 2017 22:51
// This ties in with supporting singleton custom resources that should be shared amongst templates
return _.map(_.toPairs(outputResources), ([resname, val]) => {
const isGlobal = _.has(val, '$global');
delete val.$global;
Copy link
Contributor

Choose a reason for hiding this comment

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

why do delete this?

Copy link
Contributor

Choose a reason for hiding this comment

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

was answered in slack conversation: this is not intended to appear in the cfn yaml.

Copy link
Contributor

@pauloancheta pauloancheta left a comment

Choose a reason for hiding this comment

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

👍

@pauloancheta pauloancheta merged commit 18ec063 into master Dec 10, 2017
@pauloancheta pauloancheta deleted the allow-$merge-in-cfn-Resources-node branch December 10, 2017 23:01
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.

2 participants