-
Notifications
You must be signed in to change notification settings - Fork 5k
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
refactor(notifications): use contentful package as dev dependency #26381
refactor(notifications): use contentful package as dev dependency #26381
Conversation
we only use this package for its types, so can be treated as a dev dependency
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected] |
Quality Gate passedIssues Measures |
@@ -560,6 +559,7 @@ | |||
"chalk": "^4.1.2", | |||
"chokidar": "^3.6.0", | |||
"concurrently": "^8.2.2", | |||
"contentful": "^10.8.7", |
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.
I have moved this from "dependencies" to "dev-dependencies". We only use this package for its types, so we don't strictly need this in our builds.
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.
As this is now a dev dependency, it also resolves the yarn audit
issues surrounding axios.
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.
LGTM
Builds ready [2606849]
Page Load Metrics (284 ± 255 ms)
Bundle size diffs
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #26381 +/- ##
========================================
Coverage 70.12% 70.12%
========================================
Files 1434 1434
Lines 50285 50285
Branches 13890 13890
========================================
Hits 35259 35259
Misses 15026 15026 ☔ View full report in Codecov by Sentry. |
Missing release label release-12.2.0 on PR. Adding release label release-12.2.0 on PR and removing other release labels(release-12.4.0), as PR was cherry-picked in branch 12.2.0. |
Description
We only use this package for its types, so can be treated as a dev dependency. This will also resolve the audit issue currently seen in CI.
Related issues
Fixes: CI issues, and better reflects the use case of this dependency.
Manual testing steps
Re-test notifications flow
For Notification Devs: As there is no feature announcements yet, manually change the code to point to a portfolio feature announcement. I've tested and it seems that we still correctly show a feature announcement to users.
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist