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

updated config loader to accept absolute paths to config files #3118

Merged
merged 3 commits into from Apr 26, 2018

Conversation

3 participants
@mgoldsborough
Contributor

mgoldsborough commented Apr 15, 2018

Update to support absolute paths to config files.

Commit checklist:

  • Add test cases for the changes.
  • Passed the CI test.
@coveralls

This comment has been minimized.

coveralls commented Apr 15, 2018

Coverage Status

Coverage decreased (-0.1%) to 97.119% when pulling cf76f2a on mgoldsborough:absPaths into 373b9c7 on hexojs:master.

@coveralls

This comment has been minimized.

coveralls commented Apr 15, 2018

Coverage Status

Coverage increased (+0.01%) to 97.225% when pulling e87915a on mgoldsborough:absPaths into 373b9c7 on hexojs:master.

let configPath = defaultPath;
// check for absolute config file
if (pathFn.isAbsolute(configPaths)) {

This comment has been minimized.

@JLHwung

JLHwung Apr 17, 2018

Collaborator

It can be simplified to

configPath = pathFn.isAbsolute(configPaths) ? configPaths ? pathFn.resolve(base, configPaths);

then we can check fs.existsSync in a single branch.

This comment has been minimized.

@mgoldsborough

mgoldsborough Apr 18, 2018

Contributor

Nice add. Looks good to me.

This comment has been minimized.

@mgoldsborough

mgoldsborough Apr 18, 2018

Contributor

@JLHwung do I need to implement these changes on my branch? Or can you do it as a collaborator directly via GitHub?

@mgoldsborough

This comment has been minimized.

Contributor

mgoldsborough commented Apr 26, 2018

@JLHwung I implemented your requested changes. Let me know if there is anything else.

@JLHwung JLHwung merged commit e5f16e2 into hexojs:master Apr 26, 2018

2 checks passed

codeclimate 1 fixed issue
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment