Replace fs.existsSync calls with wrapped fs.statSync#204
Replace fs.existsSync calls with wrapped fs.statSync#204jskrzypek wants to merge 3 commits intostrongloop:masterfrom
Conversation
|
Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test." |
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
lib/compiler.js
Outdated
| } catch (e) { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Can you put move this function to a shared lib such as ./lib/utils.js?
There was a problem hiding this comment.
I thought about that but didn't at first because the logic was only used more than once in compiler.js, and the function it's used in in config-loader.js, and there wasn't already a util file for shared logic.
Anyhow, I'm happy to do move it. I'll do it now.
|
Your commit message is too long too. See https://github.com/strongloop/loopback-contributor-docs/blob/master/git-commit-messages.md |
69261a4 to
a47774b
Compare
|
@superkhau Do you need me to write a test file for |
|
@jskrzypek Yes please. |
|
Tests added & green 😄 |
lib/utils.js
Outdated
|
|
||
| var fs = require('fs'); | ||
|
|
||
| exports.fileExists = fileExists; |
There was a problem hiding this comment.
Please call the method fileExistsSync to follow the naming convention used by Node core for sync methods.
|
@slnode ok to test |
|
@jskrzypek thank you for the pull request. I left few minor comments to address, the rest LGTM. |
|
@bajtos done :) |
001ddde to
5d94478
Compare
|
@bajtos I can't see why it's failing... the tests pass when I try to run |
The failures are in downstream dependencies. We are working on getting them fixed, but it's a long-term project. You can ignore them for now. |
|
Landed, thank you for the contribution! |
Fixes #201