Skip to content

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

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

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

Sorry, something went wrong.

@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.
@mgol mgol force-pushed the external-removal branch from 342c551 to 2e5ed83 Compare August 22, 2019 00:12
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.

2 participants