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.

Sign up for GitHub

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

Jump to bottom

Aspects: construct.apply should be construct.node.apply #1732

Closed
eladb opened this issue Feb 11, 2019 · 2 comments · Fixed by #1738
Closed

Aspects: construct.apply should be construct.node.apply #1732

eladb opened this issue Feb 11, 2019 · 2 comments · Fixed by #1738
Assignees
@moofish32

Comments

@eladb
Copy link
Contributor

eladb commented Feb 11, 2019

The apply method on Construct should be under node and not directly on the Construct class.
@moofish32
Copy link
Contributor

moofish32 commented Feb 11, 2019 •
edited
Loading

I'll fix this, that change came in while I was working this and I should have called that out in the review.

The change would look like:

const myConstruct = new MyConstruct(this, 'MyName', { ... });
myConstruct.node.apply(new cdk.Tag('key', 'value'));

@eladb
Copy link
Contributor Author

eladb commented Feb 11, 2019

I know. Yes, looks good to me.

moofish32 added a commit to moofish32/aws-cdk that referenced this issue Feb 12, 2019
@moofish32
fix(cdk): move apply() from Construct to ConstructNode
e415620 
This change moves the aspect `apply()` function from the `Construct` to
the `ConstructNode`. All other functionality remains the same.

BREAKING CHANGE: Tag aspects use this feature and any consumers of this
implementation must change from `myConstruct.apply( ... )` to
`myConstruct.node.apply( ... )`.

fixes aws#1732
@moofish32 moofish32 mentioned this issue Feb 12, 2019
Merged
4 tasks
eladb pushed a commit that referenced this issue Feb 12, 2019
@moofish32
fix(cdk): move apply() from Construct to ConstructNode (#1738)
642c8a6 
This change moves the aspect `apply()` function from the `Construct` to
the `ConstructNode`. All other functionality remains the same.

BREAKING CHANGE: Tag aspects use this feature and any consumers of this
implementation must change from `myConstruct.apply( ... )` to
`myConstruct.node.apply( ... )`.

fixes #1732
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@moofish32 moofish32

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(cdk): move apply() from Construct to ConstructNode moofish32/aws-cdk
2 participants
@eladb @moofish32