-
Notifications
You must be signed in to change notification settings - Fork 10
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
fixed absolute path config bug and improved workspace related specs #82
fixed absolute path config bug and improved workspace related specs #82
Conversation
@@ -17,7 +17,7 @@ module.exports = { | |||
checkCoverage: true, | |||
|
|||
statements: 85, | |||
branches: 65, | |||
branches: 70, |
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.
🏆 😎
packages/cli/lib/config.js
Outdated
// use the users provided path | ||
customConfig.workspace = workspace; | ||
} | ||
|
||
if (!fs.existsSync(workspace)) { |
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.
thinking about changing this line from this
if (!fs.existsSync(workspace)) {
to this
if (!fs.existsSync(customConfig.workspace)) {
this will make just help ensure we're validating against our own internal logic.
edit: nvm I see you left the ability to use relative still. |
Related Issue
resolves #81
Summary of Changes
Looks like our existing custom workspace wasn't robust enough, and failed to test that if there was actually any user content in the directory, it would actually get built.