-
Notifications
You must be signed in to change notification settings - Fork 244
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
chore: move replaceAssembly to @jsii/spec and expose it #3696
Conversation
/** | ||
* Replaces the old fingerprint with '***********'. | ||
* | ||
* @rmuller says fingerprinting is useless, as we do not actually check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That name (rmuller
) means nothing here... if you're gonna quote me, at least use my GitHub alias... Also I don't think I've ever said "useless", probably "currently not used", and so it was future-proofing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops... somehow it came out of the previous closed PR and I have no idea how I landed on that comment anyway. Will simply remove your alias entirely.
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
Merging (with squash)... |
1 similar comment
Merging (with squash)... |
This PR is meant to do two things:
replaceAssembly
toassembly-utils.ts
in@jsii/spec
, because it is currently being duplicated incdk-generate-synthetic-examples
. This causes problems whenever we try to changereplaceAssembly
, as we did in this commit, because we will forget to change the function incdk-generate-synthetic-examples
. The plan is to havecdk-generate-synthetic-examples
reference thereplaceAssembly
function in@jsii/spec
in a separate PR.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.