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.

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

Build: Remove the external directory, read from node_modules directly #4466

Merged
merged 1 commit into from Aug 26, 2019

Conversation

mgol
Copy link
Member

@mgol mgol commented Aug 21, 2019

Summary

Now that Sizzle is gone & we use npm, we can read from node_modules directly
and skip the setup that copies some files to the external directory.

Checklist

@mgol
Copy link
Member Author

mgol commented Aug 21, 2019

Both the Karma test run (as evidenced by this PR) and manual QUnit running via the PHP setup works with this PR.

Now that Sizzle is gone & we use npm, we can read from node_modules directly
and skip the setup that copies some files to the external directory.
Copy link
Member

@dmethvin dmethvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we have package-lock.json this is definitely a better way to do it.

@mgol
Copy link
Member Author

mgol commented Aug 25, 2019

@dmethvin package-lock.json is not committed to the repo due to various npm instability issues. That said, all 3 packages that we source from node_modules directly now are installed in specific versions in package.json and we only load their source so even without a lockfile we know exactly what we use.

@mgol mgol merged commit d7e6419 into jquery:master Aug 26, 2019
@mgol mgol deleted the external-removal branch August 26, 2019 16:53
@mgol mgol removed the Needs review label Aug 26, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Feb 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants