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

fix: options constraint has wrong type definition #1940

Merged
merged 4 commits into from
Oct 2, 2023

Conversation

nemanjastanic
Copy link
Contributor

@nemanjastanic nemanjastanic commented Sep 4, 2023

Summary

Apply correct type definitions for the constraints argument in the options function.

Fixes #1939

Requirements (place an x in each [ ])

@salesforce-cla
Copy link

salesforce-cla bot commented Sep 4, 2023

Thanks for the contribution! Before we can merge this, we need @nemanjastanic to sign the Salesforce Inc. Contributor License Agreement.

@seratch seratch marked this pull request as draft September 6, 2023 03:00
@seratch seratch added the needs info An issue that is claimed to be a bug and hasn't been reproduced, or otherwise needs more info label Sep 6, 2023
@seratch
Copy link
Member

seratch commented Sep 6, 2023

I've changed this PR to draft state. We're still trying to understand what this PR could resolve. Please refer to #1939 for the discussion.

Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

Thanks again for taking the time to send this PR! Could you check my comments?

.vscode/settings.json Outdated Show resolved Hide resolved
src/App.ts Show resolved Hide resolved
@seratch seratch added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented TypeScript-specific and removed needs info An issue that is claimed to be a bug and hasn't been reproduced, or otherwise needs more info labels Sep 11, 2023
@seratch seratch added this to the 3.x milestone Sep 11, 2023
seratch added a commit to seratch/bolt-js that referenced this pull request Sep 11, 2023
@nemanjastanic
Copy link
Contributor Author

Hey @seratch! Sorry for being super late on the update here, just got back from a lengthy vacation :D

I made the changes you asked for!

@seratch seratch marked this pull request as ready for review October 1, 2023 23:58
Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

Thank you! One last change is missing

src/App.ts Outdated Show resolved Hide resolved
@seratch
Copy link
Member

seratch commented Oct 2, 2023

I just merged the missing change in OptionsConstraints interface part. Once the CI builds pass, we'll merge this PR.

@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

Merging #1940 (4ea518a) into main (3684846) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1940   +/-   ##
=======================================
  Coverage   82.21%   82.21%           
=======================================
  Files          18       18           
  Lines        1524     1524           
  Branches      438      438           
=======================================
  Hits         1253     1253           
  Misses        175      175           
  Partials       96       96           
Files Coverage Δ
src/App.ts 83.41% <100.00%> (ø)
src/middleware/builtin.ts 84.55% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

LGTM; Thanks again for taking the time to work on this PR 🎉

@seratch seratch merged commit 44c5e01 into slackapi:main Oct 2, 2023
8 checks passed
@seratch seratch modified the milestones: 3.x, 3.14.0, 3.14.1 Oct 2, 2023
@filmaj filmaj modified the milestones: 3.14.1, 3.15.0 Nov 14, 2023
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 cla:signed TypeScript-specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TS: Constraint for options has type: "block_actions" instead of type: "block_suggestion"
3 participants