-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(cdk-test): check API compatibility #2356
Conversation
Add a check to `cdk-test` to confirm that no breaking API changes have been introduced with respect to the most recently published version. Because this will incur an NPM fetch, add a flag (`--quick`/`-q`) to disable it. Addresses #145.
// published version) | ||
if (!args.quick) { | ||
try { | ||
await shell([args["jsii-diff"], 'npm:'], { timers }); |
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.
It looks odd to have npm:
here...
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.
In a newer drop of jsii-diff we can change it to npm://
if that makes you feel better. But that is not out yet.
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.
It's more the fact that it looks like there should be something afterwards... And there's nothing...
}); | ||
|
||
const makeRed = process.stderr.isTTY ? colors.red : (x: string) => x; |
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.
❤️
@@ -27,6 +34,10 @@ export async function shell(command: string[], timers?: Timers): Promise<string> | |||
stdout.push(chunk); | |||
}); | |||
|
|||
child.stderr!.on('data', chunk => { | |||
process.stderr.write(makeRed(chunk.toString())); |
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.
chunk
might include an incomplete multi-byte unicode point, and this would effectively corrupt it 😕
Add a check to `cdk-test` to confirm that no breaking API changes have been introduced with respect to the most recently published version. Because this will incur an NPM fetch, add a flag (`--quick`/`-q`) to disable it. Addresses aws#145.
Add a check to
cdk-test
to confirm that no breaking API changeshave been introduced with respect to the most recently published
version.
Because this will incur an NPM fetch, add a flag (
--quick
/-q
) todisable it.
Addresses #145.
There is no point in merging this before a version of CDK has been
released with the new assembly version (of jsii 10) as this
tool cannot load nor validate assemblies currently published.
Pull Request Checklist
design
folderBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.