Skip to content

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

Merged
zimeg merged 1 commit intoslackapi:mainfrom
zimeg:zimeg/cli-hooks/fix-start-token-requirement
Jun 28, 2024
Merged

cli-hooks(fix): remove environment variable requirements from the start hook#1835
zimeg merged 1 commit intoslackapi:mainfrom
zimeg:zimeg/cli-hooks/fix-start-token-requirement

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 cli-hooks@1.1.1 milestone Jun 28, 2024
@zimeg zimeg self-assigned this Jun 28, 2024
@codecov
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
@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