-
Notifications
You must be signed in to change notification settings - Fork 3
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
Clean up dependency specifications #63
Conversation
2f65e4d
to
1f3d6cd
Compare
"@fortawesome/free-brands-svg-icons": "5.15.4", | ||
"@fortawesome/free-regular-svg-icons": "5.15.4", | ||
"@fortawesome/free-solid-svg-icons": "5.15.4", | ||
"@fortawesome/react-fontawesome": "0.2.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These font awesome dependencies aren't even used.
"@optimizely/react-sdk": "^2.9.2", | ||
"core-js": "^3.36.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
core-js
is not needed in 2024.
"@edx/reactifex": "^2.1.1", | ||
"@openedx/frontend-build": "14.0.3", | ||
"@openedx/paragon": "^22.2.0", | ||
"@reduxjs/toolkit": "1.8.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These deps are all specified by the learning MFE and are already in peerDependencies
so they don't also need to be in devDependencies
.
"react-router": "6.15.0", | ||
"react-router-dom": "6.15.0", | ||
"redux": "4.1.2", | ||
"regenerator-runtime": "0.13.11", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
regenerator-runtime
is not needed in 2024.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #63 +/- ##
==========================================
+ Coverage 81.36% 82.14% +0.77%
==========================================
Files 13 18 +5
Lines 220 280 +60
Branches 30 52 +22
==========================================
+ Hits 179 230 +51
- Misses 39 48 +9
Partials 2 2 ☔ View full report in Codecov by Sentry. |
1f3d6cd
to
62f174b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, thank you!
Thanks @alangsto! I will need someone on your side to merge this whenever ready. |
@bradenmacdonald I can go ahead and merge this now, but because the commit type is |
62f174b
to
5aed7e5
Compare
@alangsto Good point; changed to |
Upgrade PRs like openedx/frontend-app-learning#934 are currently blocked because this plugin is overly specifying its dependencies. There were also a number of other problems which I've fixed - see inline comments.