-
Notifications
You must be signed in to change notification settings - Fork 1
fix(cli): adds skip override flag #5
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
fix(cli): adds skip override flag #5
Conversation
sterlingwes
left a comment
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.
Looks great! Couple suggestions
Thanks for backfilling more specs ✨ 😌
src/cli/main.ts
Outdated
| .filter((_, i) => { | ||
| const originalIndex = i + 2; | ||
| return originalIndex !== skipIfEnvIndex && originalIndex !== skipIfEnvIndex + 1; | ||
| }); |
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.
splice here would be fine if we created a copy of the args argument to be safe?
const commandArgs = args.slice();
const skipIfEnvIndex = commandArgs.indexOf("--skip-if-env");
const skipIfEnv = skipIfEnvIndex > 0 ? commandArgs[skipIfEnvIndex + 1] : null;
commandArgs.splice(skipIfEnvIndex, 2);also allows for getting rid of getOptionValue alongside the filter
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.
good idea!
src/cli/main.ts
Outdated
| if (command === "override") { | ||
| if (shouldSkip(skipIfEnv)) { | ||
| 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 think we put this at the top of the run() here b/c the way it's documented it's a global CLI option and not specific to one command
Co-authored-by: Wes Johnson <[email protected]>
sterlingwes
left a comment
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.
Nice all good after these
src/cli/main.ts
Outdated
| const flagsToEnable = new Set<string>(); | ||
| args.slice(2).forEach((arg) => { | ||
|
|
||
| const argsCopy = [...args].slice(2); |
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.
slice returns a copy i believe so the spread is not necessary here
Co-authored-by: Wes Johnson <[email protected]>
Add
--skip-if-envoption to override commandSummary
Adds a new
--skip-if-env <ENV_VAR>option to theoverridecommand that skips execution when the specified environment variable is set.Usage
yarn build-flags override --skip-if-env EAS_BUILD +localFlag -otherFlagWhen
EAS_BUILDis set in the environment, the command logs a skip message and exits without modifying flags.Changes
getOptionValuehelper to extract option values from argsparseArgsto handle--skip-if-envoptionoverridecommand handler