-
Notifications
You must be signed in to change notification settings - Fork 4
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
49 init config #57
49 init config #57
Conversation
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.
Consider using const instead of let when variables don't change.
bin/capsule.js
Outdated
.description('Define the project parameters') | ||
.action(async function (options) { | ||
commander.type = 'init' | ||
let generic_questions = await parseJsonConfig(`${paths.base}/${paths.base_config}`) |
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.
probably safer to use path.resolve
for these paths
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 per your previous comment, I've updated const paths
to use:
base: path.resolve(__dirname, '../'),
So any reference to path.base
now uses the path.resolve
method. Is this what you had in mind?
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.
constructing paths with path.resolve
is a bit safer than joining with /
, especially for cross-platform support
I would play it safe on this line and the following like this:
const generic_questions = await parseJsonConfig(path.resolve(paths.base, paths.base_config))
const ci_questions = await parseJsonConfig(path.resolve(paths.base, paths.ci_config))
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.
Ahh OK gotcha, sorry misread the initial comment!
@theneverstill I've updated some of the variables to be |
@theneverstill and @grgur Have addressed the comments from above. Please review again. Thanks! |
i was faster |
@grgur haha OK, I had to push a second commit up - can you confirm again? |
Created two new commands:
init - creates a capsule.json file with the project parameters
deploy - deploys the capsule.json file