Skip to content
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

Support mkdirp to create dest path if it doesn't exists #16

Closed
wants to merge 2 commits into from

Conversation

watilde
Copy link
Member

@watilde watilde commented Apr 8, 2016

Fixes #15

@watilde watilde mentioned this pull request Apr 8, 2016
fs.statSync(name);
} catch (e) {
mkdirp.sync(path.dirname(name));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do better

mkdirp(path.dirname(name), function (err) {
  if (err) {
    fn(err);
  } else {
    fs.writeFile(name, content, fn);
  }
});

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean even without precheck with fs.stat

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems nice! Let me update it to the above you wrote.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comment! Updated codes like you said @TrySound

@TrySound
Copy link
Member

TrySound commented Apr 8, 2016

Can you provide test case?

@watilde
Copy link
Member Author

watilde commented Apr 8, 2016

Isn't this test I added enough to cover this patch? Makefile#L95-L97

@TrySound
Copy link
Member

TrySound commented Apr 8, 2016

Oh, okay. Didn't understand it's test.

@watilde
Copy link
Member Author

watilde commented Apr 8, 2016

Yeah actually we can rewrite tests to js from Makefile to make it more readable :)

@watilde watilde mentioned this pull request Apr 14, 2016
@watilde
Copy link
Member Author

watilde commented Apr 23, 2016

Merged this as f2fa45c and 08f5539.

@watilde watilde closed this Apr 23, 2016
@watilde watilde deleted the feature/mkdirp branch April 23, 2016 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants