-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement env:init, env:load and env:unload commands #2504
base: main
Are you sure you want to change the base?
Conversation
c012511
to
5e4f171
Compare
Size Change: +10.4 kB (+0.02%) Total Size: 51.8 MB
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2504 +/- ##
==========================================
- Coverage 52.59% 52.28% -0.31%
==========================================
Files 548 551 +3
Lines 20409 20607 +198
Branches 4163 4198 +35
==========================================
+ Hits 10732 10772 +40
- Misses 8837 8985 +148
- Partials 840 850 +10 ☔ View full report in Codecov by Sentry. |
5e4f171
to
1ffba14
Compare
be58276
to
e021f93
Compare
e1229ba
to
580d36e
Compare
a229442
to
e17b83a
Compare
580d36e
to
e37124b
Compare
e17b83a
to
ed27075
Compare
e37124b
to
5a70a4d
Compare
ed27075
to
546e27b
Compare
5a70a4d
to
09f1e9e
Compare
09f1e9e
to
703ba1f
Compare
703ba1f
to
dacdfdd
Compare
⏩ The changelog entry check has been skipped since the "no changelog" label is present. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed during the meeting this looks like a really cool idea and we definitely want to play with it more behind the hidden
flag. Awesome! 🚀
Few comments:
if (nonInteractive) { | ||
throw new Error("Non-interactive mode is not supported for 'eas env:init'"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just remove this flag from the command
static override flags = { | ||
...EASNonInteractiveFlag, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static override flags = { | |
...EASNonInteractiveFlag, | |
}; |
Log.log('Skipping adding the direnv hook to the shell config'); | ||
Log.log('You may need to add the direnv hook to your shell config manually.'); | ||
Log.log('Learn more: https://direnv.net/docs/hook.html'); | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return; | |
throw new Error |
to make it exit with non-zero code in such case
Log.log('The direnv hook is already present in the shell config'); | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would throw error here as well
} else { | ||
Log.log("Unable to determine the user's shell"); | ||
Log.log('You may need to add the direnv hook to your shell config manually.'); | ||
Log.log('Learn more: https://direnv.net/docs/hook.html'); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we also need to throw an error here
throw new Error('Your Linux distribution is not supported by this script.'); | ||
} | ||
} else { | ||
Log.log(`Your platform (${platform}) is not supported by this script.`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Log.log(`Your platform (${platform}) is not supported by this script.`); | |
throw new Error(`Your platform (${platform}) is not supported by this script.`); |
.filter((variable: EnvironmentVariableFragment) => variable.value !== null) | ||
.map((variable: EnvironmentVariableFragment) => { | ||
return `${variable.name}=${variable.value}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.filter((variable: EnvironmentVariableFragment) => variable.value !== null) | |
.map((variable: EnvironmentVariableFragment) => { | |
return `${variable.name}=${variable.value}`; | |
.filter((variable => variable.value !== null) | |
.map((variable) => { | |
return `${variable.name}=${variable.value}`; |
@@ -24,7 +24,7 @@ export default class EnvironmentValuePull extends EasCommand { | |||
...EASNonInteractiveFlag, | |||
path: Flags.string({ | |||
description: 'Path to the result `.env` file', | |||
default: '.env.local', | |||
default: '.env.eas.local', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH I would stick with .env.local
as default, I believe that's what the average person expects, and .env.local
works as well with Expo SDK by default but .env.eas.local
doesn't.
Why
Commands streamlining usage of environment variables
eas env:init
- install direnv, set up .envrc file and .gitignoreeas env:load --environment production
- pulls envvars from given environment and loads themeas env:unload
- unloads envvarsTest Plan
Tested manually