Skip to content
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

Bump the oldest supported version of node-slack-sdk to support modals for sure (cherry-picking #287) #322

Merged
merged 1 commit into from
Nov 28, 2019

Conversation

seratch
Copy link
Member

@seratch seratch commented Nov 27, 2019

Summary

With @slack/web-api 5.0.0, Block Kit in modals doesn't work in Bot apps as necessary ones are not available in such old versions. #306 (comment) is an issue caused by that.

Block Kit in modals are available in @slack/web-api 5.2.1.

This pull request cherry-picked only the change resolving it from @PerStirpes 's PR #287 .

Requirements (place an x in each [ ])

@seratch seratch added the bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented label Nov 27, 2019
@seratch seratch self-assigned this Nov 27, 2019
@codecov
Copy link

codecov bot commented Nov 27, 2019

Codecov Report

Merging #322 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #322   +/-   ##
=======================================
  Coverage   73.71%   73.71%           
=======================================
  Files           7        7           
  Lines         506      506           
  Branches      145      145           
=======================================
  Hits          373      373           
  Misses        101      101           
  Partials       32       32

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb3b436...1018cc1. Read the comment docs.

Copy link
Member

@stevengill stevengill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

New functionality being added in a minor release is totally acceptable. It wouldn't warrant a major version bump as nothing broke.

It would have been nice if we updated the dependencies alongside adding the functionality. Good to keep in mind next time. Wonder if we can tweak our testing env to catch something like this.

@seratch seratch merged commit 1d9cbaf into slackapi:master Nov 28, 2019
@seratch seratch deleted the pr-287-cherry-pick branch November 28, 2019 01:06
@seratch seratch mentioned this pull request Nov 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants