feat: remove deno from installation scripts#189
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #189 +/- ##
==========================================
- Coverage 62.97% 62.95% -0.03%
==========================================
Files 212 212
Lines 21782 21782
==========================================
- Hits 13718 13713 -5
- Misses 7003 7006 +3
- Partials 1061 1063 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
🗣️ The "Lints and Unit tests" check is pending because this workflow name was changed to "Lints and Tests" in this PR. Before merging we can update the required checks if that's alright! |
WilliamBergamin
left a comment
There was a problem hiding this comment.
Nice job 🚀 I don't have much to add, we can always fix forward
| - [Bolt for JavaScript](/tools/bolt-js/getting-started) | ||
| - [Bolt for Python](/tools/bolt-python/getting-started) | ||
| - [Deno Slack SDK](/tools/deno-slack-sdk/guides/getting-started) |
| Write-Host "`nUse of the Slack CLI should comply with the Slack API Terms of Service:" | ||
| Write-Host " https://slack.com/terms-of-service/api" |
| fi | ||
| ;; | ||
| *) | ||
| >&2 echo -e "\x1b[1m⚠️ Warning: An unknown flag '$1' was passed to the Slack CLI installation script!\x1b[0m" |
There was a problem hiding this comment.
Praise the user experience improvement 🙏
| delay 0.2 "💾 Successfully downloaded Slack CLI v$LATEST_SLACK_CLI_VERSION to $(home_path "$slack_cli_install_dir/slack-cli.tar.gz")" | ||
|
|
||
| delay 0.3 "📦 Extracting the Slack CLI command binary to $(home_path $slack_cli_bin_path)" | ||
| delay 0.3 "📦 Extracting the Slack CLI command binary to $(home_path "$slack_cli_bin_path")" |
| sleep 0.1 | ||
| feedback_message |
There was a problem hiding this comment.
Out of scope, but is there a way to toggle these sleep off to improve the CI experience?
There was a problem hiding this comment.
I don't believe so right now, but this is a good idea to follow up on 🙏
IIRC we've discussed environment variable options but I forget at the moment if we settled on something? I'll also be thinking about this!
zimeg
left a comment
There was a problem hiding this comment.
@WilliamBergamin Thanks for the kind review! I'm hoping to merge this for next release but will hold off right now.
Also - huge thanks to @lukegalbraithrussell for the branch previews - I left a few comments below to make sure links follow as expected 👁️🗨️
| sleep 0.1 | ||
| feedback_message |
There was a problem hiding this comment.
I don't believe so right now, but this is a good idea to follow up on 🙏
IIRC we've discussed environment variable options but I forget at the moment if we settled on something? I'll also be thinking about this!
|
|
||
| The automated installer attempts to add the Slack CLI to a known directory in the path: `/usr/local/bin`. | ||
|
|
||
| Some machines protect this directory and elevated access might be needed for updated permissions. We do not recommend using `sudo` with the automated installer since unexpected side effects might place downloads in the wrong spot. |
There was a problem hiding this comment.
i'll sudo if i want to!
lukegalbraithrussell
left a comment
There was a problem hiding this comment.
nits of words but docs look good both here and on the preview!
Co-authored-by: Luke Russell <31357343+lukegalbraithrussell@users.noreply.github.com>
Co-authored-by: Luke Russell <31357343+lukegalbraithrussell@users.noreply.github.com>
zimeg
left a comment
There was a problem hiding this comment.
@lukegalbraithrussell I appreciate the review!
I'm going to keep reading the docs and might land a few small changes but will hold off on merging this for the moment 🫡
Co-authored-by: Haley Elmendorf <31392893+haleychaas@users.noreply.github.com>
|
@WilliamBergamin @lukegalbraithrussell @haleychaas Thank y'all for the reviews in this change 🙏 ✨ Now is seeming like an alright time to merge this, so let's! I am wondering if we want to avoid publishing the docs updates for now but will open discussion on this elsewhere 🤖 I've also made a note about the faster CI improvements to follow for this script, and am hoping we can continue follow ups of that and #188 soon! 🚢 Marking the calendar to publish these scripts on September 1 upcoming 📆 👾 |
|
🗣️ The changes of this PR are unrelated to CLI command behavior and are scheduled to be uploaded separate of when this release goes public IIRC. Changing it to a |
| jobs: | ||
| lint-test: | ||
| name: Lints and Unit tests | ||
| name: Lints and Tests |
There was a problem hiding this comment.
@mwbrooks I'm so glad we're testing installation scripts here now too! It might be excessive since even unrelated changes exercise this, but I like that the confidence it brings 🤖 ✨
Changelog
The
denoruntime is no longer installed with the installation script. We recommend installing deno separately and configuring CI systems as needed for app development (such as for ROSI apps).Summary
This PR removes
denofrom the installation scripts for improvedboltsupport. Also included are tests for the unix installation scripts in CI.Preview
install.mov
Reviewers
🦕 An upstream version can be downloaded with these commands:
Notes
Requirements