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

Added mkdtemp and sorted exports list. #41

Closed
wants to merge 2 commits into from

Conversation

EvanCarroll
Copy link

No description provided.

@EvanCarroll
Copy link
Author

Closes out #40

@coveralls
Copy link

coveralls commented Oct 10, 2016

Coverage Status

Coverage remained the same at 92.0% when pulling 07e9200 on EvanCarroll:master into a0d3e40 on normalize:master.

'symlink',
'lstat',
'mkdir',
'mkdtemp',
Copy link
Member

@haoxins haoxins Oct 11, 2016

Choose a reason for hiding this comment

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

Personally, I'd like to split add mkdtemp and sort the list to different PRs ((_))

Copy link
Author

Choose a reason for hiding this comment

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

Sure, in retrospect..

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree too, but I think this is good enough for now...

@LinusU
Copy link
Contributor

LinusU commented Oct 11, 2016

@EvanCarroll Could you please add a test as well? 💌

@EvanCarroll
Copy link
Author

This module not having a testing a suite is an issue in and of itself. I don't have the time nor concern to tackle that issue. Not a single function, afaik, is testing in mz/fs.js

@LinusU
Copy link
Contributor

LinusU commented Oct 12, 2016

Tests exists: https://github.com/normalize/mz/blob/master/test/mz.js#L4-L48

I think that this should just be adding ~6 lines of code, but maybe I'm wrong?

@RyanZim
Copy link
Contributor

RyanZim commented Apr 8, 2017

Looks like this was fixed in 4dd2cfe.

@LinusU
Copy link
Contributor

LinusU commented Apr 10, 2017

Cool 👌

Feel free to send a new pull request which sorts all the lists :)

@LinusU LinusU closed this Apr 10, 2017
@RyanZim RyanZim mentioned this pull request Apr 10, 2017
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.

None yet

5 participants