feat: add Slack CLI command runner workflow and installer composite action #489
Conversation
zimeg
left a comment
zimeg
left a comment
There was a problem hiding this comment.
@ewanek1 This is an incredible set of changes from the prior PR! I'm marking it as approved with all things moving in a good direction 🎉
Handfuls of comments and thoughts and suggestions follow that I do think we'll want to address before a public release, but these don't have to block prereleases IMHO 🚂 💨
The comments on inputs and outputs would be most curious to discuss since these are difficult to change after release, but I'm super curious what you think about other comments as well!
Huge thanks for working on all of this and maintaining support for setups of all kind 🎁 ✨
There was a problem hiding this comment.
⭐ praise: We've discussed this setup and I think the separated action definition is a nice pattern to use for the CLI technique! Thanks for encouraging this!
👁️🗨️ thought: The exact calling syntax escapes me but for releases it'd be so interesting to find this pattern in calling workflows:
- uses: slackapi/slack-github-action/cli@v2
with:
command: version| - name: Add Slack CLI to PATH (Linux/macOS) | ||
| if: runner.os != 'Windows' | ||
| shell: bash | ||
| run: echo "$HOME/.slack/bin" >> "$GITHUB_PATH" | ||
|
|
||
| - name: Add Slack CLI to PATH (Windows) | ||
| if: runner.os == 'Windows' | ||
| shell: pwsh | ||
| run: Add-Content -Path $env:GITHUB_PATH -Value "$env:USERPROFILE\.slack\bin" |
There was a problem hiding this comment.
👾 question: Are these steps needed just if the cache was found? Unsure if the installation script has similar effects or if this could be skipped...
There was a problem hiding this comment.
Ah yes seems like I would need to move the action to the root level so slack-github-action/cli/action.yml. Instead of having it under .github/resources/.actions
| - name: Cache Slack CLI | ||
| id: cache-cli | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: | | ||
| ${{ runner.os == 'Windows' && format('{0}/AppData/Local/slack-cli', env.USERPROFILE) || '~/.slack/bin' }} | ||
| key: slack-cli-${{ runner.os }}-${{ runner.arch }}-${{ inputs.cli_version }} |
There was a problem hiding this comment.
💰 praise: Super nice to be landing this in a first release. While installation is somewhat fast I think restoring from cache is often faster!
There was a problem hiding this comment.
🤖 question: I notice the ~ is used when checking this path but the $HOME variable is used following. Would using the $HOME variable here be possible too?
| @@ -0,0 +1,130 @@ | |||
| name: Slack CLI Installation and Command Runner | |||
There was a problem hiding this comment.
| name: Slack CLI Installation and Command Runner | |
| name: "Slack: Run a CLI command" |
🌲 suggestion: To match the action.yml file for other techniques!
| @@ -0,0 +1,130 @@ | |||
| name: Slack CLI Installation and Command Runner | |||
| description: Download and cache the Slack CLI and run the input command | |||
There was a problem hiding this comment.
| description: Download and cache the Slack CLI and run the input command | |
| description: "Install the Slack CLI to run a command" |
🌞 suggestion: Perhaps idea on wording - I'm hoping to leave magic behind the scenes with this change but am not sure of the verbiage.
There was a problem hiding this comment.
I like this! It's less verbose and more user-centric imo 🙇♀️
.github/workflows/cli-runner.yml
Outdated
| timeout-minutes: 5 | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 |
| test-all: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
| - uses: actions/checkout@v4 | |
| - name: Checkout the current code | |
| uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 |
👁️🗨️ suggestion: Guards against unexpected changes upstream!
| - name: Run slack version | ||
| id: version | ||
| uses: ./.github/resources/.actions/cli-runner | ||
| with: | ||
| command: "version" |
There was a problem hiding this comment.
🧪 suggestion: It might be interesting to run this test across multiple runners - ubuntu, mac, and windows?
| - name: Long command with flags | ||
| id: long-command | ||
| uses: ./.github/resources/.actions/cli-runner | ||
| with: | ||
| command: 'doctor --help --experiment string' | ||
| cli_version: "latest" | ||
| env: | ||
| SLACK_SERVICE_TOKEN: ${{ secrets.SLACK_SERVICE_TOKEN }} |
There was a problem hiding this comment.
🧪 suggestion(non-blocking): We should follow up with a test that uses both the --app and token value to make sure all things reach the Slack API as expected:
manifest --app $SLACK_APP_ID
🗣️ ramble: I'm not certain if this environment variable can be gathered from the env but that'd be a nice thing to find in test I hope too?
There was a problem hiding this comment.
Agreed similar to what I have for verbose.
There was a problem hiding this comment.
📝 suggestion: Following a prior comment on documentation and example, this might be nice to include in our example workflows?
🦕 thought: I can imagine running the CLI technique on merges to main as well to keep our testing app current:
🔗 https://github.com/slackapi/slack-github-action/tree/main/.github/resources/.slack
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## ewanek1-slack-cli-command-runner-v2 #489 +/- ##
====================================================================
Coverage 99.86% 99.86%
====================================================================
Files 7 7
Lines 722 722
====================================================================
Hits 721 721
Misses 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
This PR builds upon #488 and #486. It continues iterating on the GitHub composite action and workflow that runs Slack CLI commands directly in CI/CD.
Notes on Major Changes
Built a testing workflow that triggers on a pull request
Updated workflow to be reusable and callable by users in their own repositories
Wrote documentation
Included an app_id input option
Added support for Windows
Requirements