-
Notifications
You must be signed in to change notification settings - Fork 85
Remove (some) unsused dependencies #1383
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
base: master
Are you sure you want to change the base?
Remove (some) unsused dependencies #1383
Conversation
The test runs are green but eslint suggests adding some (indirect?) dependencies back. |
I think they are not indirect... they are just not covered by tests (?) For example in helpers we need the dep to check if it is really a ThingModel... |
Do you think it is worth removing those unused dependencies in this PR? |
I think it is okay, I have a doubt for |
In my understanding @jspm/core is needed to integrate the lastest changes from this PR. I'd keep it until we try to migrate directly to a new version of esbuild-plugin-polyfill-node which should be https://www.npmjs.com/package/esbuild-plugins-node-modules-polyfill . |
I added @jspm/core@2.1.0 back in browser-bundle The remaining changes are minimal. I let others decide whether it is still worth to merge... |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1383 +/- ##
==========================================
+ Coverage 77.06% 77.29% +0.22%
==========================================
Files 79 79
Lines 15290 15290
Branches 1442 1442
==========================================
+ Hits 11783 11818 +35
+ Misses 3483 3447 -36
- Partials 24 25 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I used
npm-check
to identify unsued dependencies. Unfortunately, it gives lots of false positives, and it ends up with trial and error 🤷♂️Anyhow, I managed to get rid of some ..