-
Notifications
You must be signed in to change notification settings - Fork 399
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
Upgrade dependencies #2360
Upgrade dependencies #2360
Conversation
tests pass locally with new deps installed |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2360 +/- ##
=======================================
Coverage 92.59% 92.59%
=======================================
Files 36 36
Lines 7472 7472
Branches 653 653
=======================================
Hits 6919 6919
Misses 545 545
Partials 8 8 ☔ View full report in Codecov by Sentry. |
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 💯 but may I suggest waiting until the number of downloads for @slack/socket-mode v2.0.3
rises slightly before merging/releasing these changes
socket-mode is a critical part of this project, if there is an issue with it it would be nice to catch it before releasing it here 🤔
holding off on merging for now based on the above ^ |
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.
Bumping the @slack
dependencies of bolt always brings a question or two...
Otherwise, feel free to merge when the time is right! 🙏
And I tested these changes with a few typescript
and javascript
projects and found the builds are alright as well, though I'm always hoping to find more ways to test future changes 👀
"@slack/types": "^2.13.0", | ||
"@slack/web-api": "^7", | ||
"axios": "^1.7.4", | ||
"@slack/web-api": "^7.8.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.
Would this bump make this change a semver:minor
because new features are introduced? 🤔
I'm thinking we should've had this set to the latest semver:minor
anyways since features of @slack/web-api
are exposed from @slack/bolt
- such as the assistant
APIs released in @slack/web-api@7.5.0
being required since @slack/bolt@4.0.0
- but let me know what you think! 🔍
@hello-ashleyintech got a few thousand download of socket mode 2.0.3 with no reported issues, I think we can safely merge this |
@hello-ashleyintech @WilliamBergamin Jumping in on this winterish week to merge this and will tag it as @hello-ashleyintech Thanks a ton for making these changes upstream and here! Huge lifts! 💪 ✨ |
Summary
This PR updates the following dep minimums to most recent version to avoid security vulns:
@slack/oauth
to3.0.2
@slack/socket-mode
to2.0.3
@slack/web-api
to7.8.0
axios
to1.7.8
Requirements (place an
x
in each[ ]
)