-
Notifications
You must be signed in to change notification settings - Fork 6
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
Some tweaks to CLI configuration #323
Conversation
- Updates CLI help text to explain how to pass args through to underlying utility. - Displays help text of underlying utility (in addition to general env help) if -h is passed to the yarn 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.
pulled and tested locally, works great. thank you!
I re-ordered some of the test:
commands in package.json and updated test:watch
to use the passthrough syntax.
// Return all options with proper cascading: | ||
// 1. CLI options have highest precedence | ||
// 2. CMS feature flags next | ||
// 3. ENV file vars last |
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 this comment (or something to that effect) is valuable to keep even if it is technically self-documented in the code
...envVars, | ||
...cliOptions, | ||
} | ||
// For now, don't fetch CMS feature flags for tests. Will fail CI. |
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.
not a blocker for this, just curious what failures you were seeing? is it related to fetching an env behind the VA network through GHA (gh-provided vs self-hosted runners), or something else?
Description
-h
is passed to the yarn script.These changes allowed the removal of a few yarn scripts:
lint:staged:test-modified-files
: no longer need this because we can pass the necessary commands through toyarn test
now and we couldn't before. (Note: before the creation of this now-deleted file, we were simply runningjest
, but this yarn script was created so as to be able to run the env-loader before running jest. We can now accomplish both by runningyarn test
and passing arguments through to jest.)test:watch
: can now runyarn test -- --watch
test:update-snapshots
: can now runyarn test -- -u
(Note: We could keeptest:watch
andtest:update-snapshots
if we wanted that layer of abstraction, but I'm not sure it's necessary.)Testing done
Manual testing of yarn scripts
QA steps
jest
,next
)-h
and see both general env help and underlying-utility help (e.g.yarn test -h
)