- Notifications
You must be signed in to change notification settings - Fork20.6k
Build: Restore the external directory#4865
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Merged
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or 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
Injquerygh-4466, we removed the `external` directory in favor of loading some filesdirectly from `node_modules`. This works fine locally but when deploying codefor tests, this makes it impossible to not deploy `node_modules` as well. Toavoid the issue, this change restores usage of the `external` directory.One change is that we no longer commit this directory to the repository, itsonly purpose is to have clear isolation from `node_modules`.Refjquerygh-4466
Krinkle approved these changesMar 24, 2021
mgol added a commit to mgol/jquery-migrate that referenced this pull requestMar 24, 2021
Inc12e1ab, we removed the `external` directoryin favor of loading some files directly from `node_modules`. This works finelocally but when deploying code for tests, this makes it impossible to notdeploy `node_modules` as well. To avoid the issue, this change restores usageof the `external` directory.One change is that we no longer commit this directory to the repository, itsonly purpose is to have clear isolation from `node_modules`.Refjquery/jquery#4865
mgol added a commit to mgol/jquery-migrate that referenced this pull requestMar 24, 2021
Inc12e1ab, we removed the `external` directoryin favor of loading some files directly from `node_modules`. This works finelocally but when deploying code for tests, this makes it impossible to notdeploy `node_modules` as well. To avoid the issue, this change restores usageof the `external` directory.One change is that we no longer commit this directory to the repository, itsonly purpose is to have clear isolation from `node_modules`.Refjquery/jquery#4865
mgol added a commit to mgol/jquery-migrate that referenced this pull requestMar 24, 2021
Inc12e1ab, we removed the `external` directoryin favor of loading some files directly from `node_modules`. This works finelocally but when deploying code for tests, this makes it impossible to notdeploy `node_modules` as well. To avoid the issue, this change restores usageof the `external` directory.One change is that we no longer commit this directory to the repository, itsonly purpose is to have clear isolation from `node_modules`.Refjquery/jquery#4865
timmywil approved these changesMar 24, 2021
external directorymgol added a commit to jquery/jquery-migrate that referenced this pull requestMar 25, 2021
Inc12e1ab, we removed the `external` directoryin favor of loading some files directly from `node_modules`. This works finelocally but when deploying code for tests, this makes it impossible to notdeploy `node_modules` as well. To avoid the issue, this change restores usageof the `external` directory.One change is that we no longer commit this directory to the repository, itsonly purpose is to have clear isolation from `node_modules`.Refjquery/jquery#4865Closesgh-419
mgol added a commit to mgol/jquery that referenced this pull requestApr 8, 2021
That package was missed injquerygh-4865 as it only broke browsers needing thepolyfill which is just IE at the moment. Thus, it broke Core tests in IE only.Refjquerygh-4865
2 tasks
mgol added a commit to mgol/jquery that referenced this pull requestApr 13, 2021
All the other files were already taken from the external directory.The fact core-js was taken from node_modules broke IE core tests on TestSwarm.Refjquerygh-4865Refjquerygh-4870(partially cherry picked from345cd22)
Sign up for freeto 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.
Summary
Ingh-4466, we removed the
externaldirectory in favor of loading some filesdirectly from
node_modules. This works fine locally but when deploying codefor tests, this makes it impossible to not deploy
node_modulesas well. Toavoid the issue, this change restores usage of the
externaldirectory.One change is that we no longer commit this directory to the repository, its
only purpose is to have clear isolation from
node_modules.Refgh-4466
Checklist
New tests have been added to show the fix or feature worksIf needed, a docs issue/PR was created athttps://github.com/jquery/api.jquery.com