Enable CLI installation - #1253
Conversation
| if ( process.env.NODE_ENV === 'development' ) { | ||
| await installCLIOnWindows(); | ||
| } | ||
| await installCLIOnWindows(); |
There was a problem hiding this comment.
Should we use a feature flag instead?
There was a problem hiding this comment.
Could you elaborate? At this point, we're enabling the feature for all users.
From a technical standpoint, we went with the "cheap" process.env.NODE_ENV === 'development' approach previously because the only existing logic we have for loading feature flags lives in the renderer, whereas this feature lives in the main process and runs on initialization.
There was a problem hiding this comment.
Could you elaborate? At this point, we're enabling the feature for all users.
If we remove this code and do not use the concept of feature flag, we will release this feature to all users with the next release on Monday. I wanted to confirm if it's what we are planning.
There was a problem hiding this comment.
Ah, I see. I overlooked the fact that the 1.4.1 release target date is next Monday. Give me a minute to think about how to approach this…
There was a problem hiding this comment.
I've added a feature gate based on the app version that enables the CLI installation for development and beta builds.
gcsecsey
left a comment
gcsecsey
left a comment
There was a problem hiding this comment.
LGTM, thanks for the detailed reasoning behind these changes above. 🙌
4 participants
Related issues
Proposed Changes
Update the conditionals limiting the CLI installation feature so they are also enabled in development and beta builds of Studio.
Testing Instructions
Code review should suffice
Pre-merge Checklist