chore: move API compatiblity check to final build step #2911
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Motivation: taking it out of the per-package 'test' step has the following benefits: - the build+test cycle is quicker, because we avoid unnecessary downloads of old versions of the package from NPM. - we avoid quadratic complexity of downloads, where packages deep in the dependency tree get downloaded over and over again. - there is a clear moment and location where the check is done, which can be easily disabled. There is still accidental quadratic complexity in the current implementation which cannot be avoided without updating jsii-diff: although all downloads are done only once, every package's JSII assembly is still loaded individually, and all dependencies are reloaded every time. Loading assemblies is not as cheap as you'd think, leading many packages to take multiple seconds to load (~4s per package) and so the whole process still takes multiple minutes.
eladb
approved these changes
Jun 18, 2019
| mkdir -p $tmpdir | ||
|
|
||
| echo "Installing from NPM..." >&2 | ||
| (cd $tmpdir && npm install --prefix $tmpdir ${package_names[@]}) |
There was a problem hiding this comment.
In a tempdir? Don't care.
There was a problem hiding this comment.
Also, there is no package.json so where would it save :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation: taking it out of the per-package 'test' step has the
following benefits:
downloads of old versions of the package from NPM.
in the dependency tree get downloaded over and over again.
which can be easily disabled.
There is still accidental quadratic complexity in the current
implementation which cannot be avoided without updating jsii-diff:
although all downloads are done only once, every package's JSII
assembly is still loaded individually, and all dependencies are
reloaded every time.
Loading assemblies is not as cheap as you'd think, leading many
packages to take multiple seconds to load (~4s per package) and
so the whole process still takes multiple minutes.
Pull Request Checklist
designfolderBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.