-
Notifications
You must be signed in to change notification settings - Fork 31
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
ACNA-621 - init --import #133
Conversation
|
Codecov Report
@@ Coverage Diff @@
## master #133 +/- ##
=======================================
Coverage 97.85% 97.85%
=======================================
Files 18 18
Lines 466 466
Branches 56 56
=======================================
Hits 456 456
Misses 10 10 Continue to review full report at Codecov.
|
@shazron or @purplecabbage ready to review! |
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.
A couple nits, looks good though
if (flags.import) { | ||
const config = loadConfigFile(flags.import).values | ||
|
||
projectName = config.name // must be defined |
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.
There is some weirdness here ...
If I specify a project path as an arg to init, then the directory is created, and becomes cwd.
If a config file is used, then the project name comes from there, but no directory is created, and project is placed in cwd.
Ultimately this is probably just something we need to document..
expect(TheCommand.flags.import.char).toBe('i') | ||
}) | ||
|
||
test('args', async () => { |
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.
We previously removed tests that were this tightly tied to the implementation.
Test should only verify that arg has description, and default, but not what the values actually are.
Description
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: