feat: 'env' command no longer prompts for an app for Bolt projects#456
feat: 'env' command no longer prompts for an app for Bolt projects#456
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #456 +/- ##
=======================================
Coverage 70.95% 70.96%
=======================================
Files 220 220
Lines 18450 18466 +16
=======================================
+ Hits 13092 13104 +12
- Misses 4181 4183 +2
- Partials 1177 1179 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zimeg
left a comment
There was a problem hiding this comment.
🗣️ Notes to self and others which is much the same-
| // isHostedRuntime returns true if the local manifest is for an app that uses | ||
| // the Deno Slack SDK function runtime. | ||
| // | ||
| // It defaults to false when the manifest cannot be fetched, which directs the | ||
| // command to use the project ".env" file. Otherwise the API is used. | ||
| func isHostedRuntime(ctx context.Context, clients *shared.ClientFactory) bool { | ||
| manifest, err := clients.AppClient().Manifest.GetManifestLocal(ctx, clients.SDKConfig, clients.HookExecutor) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| return manifest.IsFunctionRuntimeSlackHosted() || manifest.IsFunctionRuntimeLocal() | ||
| } |
There was a problem hiding this comment.
🔭 note: I'm not thrilled with the placement of this logic but we might want to keep it scoped to the env commands at this time?
There was a problem hiding this comment.
I'm surprised we don't have a helper function already, but I agree let's just scope it to env for now.
mwbrooks
left a comment
There was a problem hiding this comment.
✅ Yea, this is such a nice experience! 🎉 The note about adding creating the .env is also very helpful.
🧪 Works well for Deno Deployed, Deno Local, and Bolt Local apps.
✏️ I changed the label/title from bug/fix to enhancement/feat. This is a new feature for Bolt apps and I feel like it's an enhancement on the previous experience that required the app to be installed.
| // isHostedRuntime returns true if the local manifest is for an app that uses | ||
| // the Deno Slack SDK function runtime. | ||
| // | ||
| // It defaults to false when the manifest cannot be fetched, which directs the | ||
| // command to use the project ".env" file. Otherwise the API is used. | ||
| func isHostedRuntime(ctx context.Context, clients *shared.ClientFactory) bool { | ||
| manifest, err := clients.AppClient().Manifest.GetManifestLocal(ctx, clients.SDKConfig, clients.HookExecutor) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| return manifest.IsFunctionRuntimeSlackHosted() || manifest.IsFunctionRuntimeLocal() | ||
| } |
There was a problem hiding this comment.
I'm surprised we don't have a helper function already, but I agree let's just scope it to env for now.
|
@mwbrooks The framing is much better as an improvement of what landed before 🤓 ✨ Let's get this merged for a few more changes around the |
Changelog
Summary
This PR defaults to project variables of
envcommands for Bolt applications while requiring app selection for ROSI projects 🥀Resolves confusion of the scope these commands are applied to!
Preview
Bolt
ROSI
Local app
Deployed app
Reviewers
Please confirm the following works for various cases:
Notes
This follows changes of #437 and #451 which both "unlock" the
envcommands for local ROSI apps but I am unsure if we want to keep this change?Requirements