chore(core): remove all references to BundlingDockerImage in the public API #13814
Conversation
A previous commit - ad01099 - deprecated BundlingDockerImage in favour of DockerImage. However, there are still uses of BundlingDockerImage that remain. Since bundling is still experimental, swap all uses of BundlingDockerImage and replace with DockerImage. Motivation No non-deprecated public API can reference a deprecated type as part of CDKv2. BREAKING CHANGE: The type of the `image` property in `BundlingOptions` is changed from `BundlingDockerImage` to `DockerImage`. * **core:** The return type of the `DockerImage.fromBuild()` API is changed from `BundlingDockerImage` to `DockerImage`.
|
|
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
| // timestamps. | ||
| const hash = FileSystem.fingerprint(path, { extraHash: JSON.stringify(options) }); | ||
| return new BundlingDockerImage(tag, hash); | ||
| DockerImage.fromBuild(path, options) as BundlingDockerImage; |
There was a problem hiding this comment.
What's doing on here? Seems like the return statement is missing and also the downcast seems hazardous.
@jogold any comments?
There was a problem hiding this comment.
The original API returned BundlingDockerImage, hence returning the same to keep it backwards compat.
The alternative is to mark the API return type in the method signature and let typescript do the downcast.
There was a problem hiding this comment.
I am confused. This code does not return anything (there is no return statement). I don't see how this alternative can safely work because fromBuild does not return a BuildingDockerImage, is it?
There was a problem hiding this comment.
Oops, it should have a return statement. That's a miss. I'll submit a fix.
fromBuild() returns a DockerImage which is a subtype of BundlingDockerImage, so downcasting should be fine, no?
…ic API (aws#13814) A previous commit - ad01099 - deprecated BundlingDockerImage in favour of DockerImage. However, there are still uses of BundlingDockerImage that remain. Since bundling is still experimental, swap all uses of BundlingDockerImage and replace with DockerImage. Motivation No non-deprecated public API can reference a deprecated type as part of CDKv2. BREAKING CHANGE: The type of the `image` property in `BundlingOptions` is changed from `BundlingDockerImage` to `DockerImage`. * **core:** The return type of the `DockerImage.fromBuild()` API is changed from `BundlingDockerImage` to `DockerImage`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…ic API (aws#13814) A previous commit - ad01099 - deprecated BundlingDockerImage in favour of DockerImage. However, there are still uses of BundlingDockerImage that remain. Since bundling is still experimental, swap all uses of BundlingDockerImage and replace with DockerImage. Motivation No non-deprecated public API can reference a deprecated type as part of CDKv2. BREAKING CHANGE: The type of the `image` property in `BundlingOptions` is changed from `BundlingDockerImage` to `DockerImage`. * **core:** The return type of the `DockerImage.fromBuild()` API is changed from `BundlingDockerImage` to `DockerImage`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
A previous commit - ad01099 - deprecated BundlingDockerImage in favour
of DockerImage.
However, there are still uses of BundlingDockerImage that remain. Since
bundling is still experimental, swap all uses of BundlingDockerImage and
replace with DockerImage.
Motivation
No non-deprecated public API can reference a deprecated type as part of
CDKv2.
BREAKING CHANGE: The type of the
imageproperty inBundlingOptionsis changed from
BundlingDockerImagetoDockerImage.DockerImage.fromBuild()API ischanged from
BundlingDockerImagetoDockerImage.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license