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

cli-hooks(fix): remove environment variable requirements from the start hook #1835

Merged

Conversation

zimeg
Copy link
Member

@zimeg zimeg commented Jun 28, 2024

Summary

This PR removes validation of environment variables from the start hook since it's not always true that these tokens are required to run an app. For instance, an app token isn't needed when running using HTTP Request URLs!

Similar assertions were removed and don't exist in slackapi/python-slack-hooks: slackapi/python-slack-hooks#27

Requirements

@zimeg zimeg added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch pkg:cli-hooks applies to `@slack/cli-hooks` labels Jun 28, 2024
@zimeg zimeg added this to the [email protected] milestone Jun 28, 2024
@zimeg zimeg self-assigned this Jun 28, 2024
Copy link

codecov bot commented Jun 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.81%. Comparing base (9e20ca3) to head (859fe15).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1835      +/-   ##
==========================================
- Coverage   81.85%   81.81%   -0.05%     
==========================================
  Files          35       35              
  Lines        7783     7764      -19     
  Branches      318      318              
==========================================
- Hits         6371     6352      -19     
  Misses       1400     1400              
  Partials       12       12              
Flag Coverage Δ
cli-hooks 94.94% <ø> (-0.14%) ⬇️
cli-test 54.16% <ø> (ø)
oauth 76.53% <ø> (ø)
socket-mode 59.59% <ø> (ø)
web-api 96.55% <ø> (ø)
webhook 95.27% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@srajiang srajiang self-requested a review June 28, 2024 00:28
@zimeg
Copy link
Member Author

zimeg commented Jun 28, 2024

@srajiang thank you for the fast review! I'll merge this now but hold off on a release for a bit more testing on errors and exits 🚀

@zimeg zimeg merged commit e527b43 into slackapi:main Jun 28, 2024
30 checks passed
@zimeg zimeg deleted the zimeg/cli-hooks/fix-start-token-requirement branch June 28, 2024 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented pkg:cli-hooks applies to `@slack/cli-hooks` semver:patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants