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

Node.js 0.8 compatibility and full async behavior #5

Closed
wants to merge 11 commits into from

Conversation

wdavidw
Copy link

@wdavidw wdavidw commented Jul 26, 2012

Hope those changes will make it through. I use open-uri in mecano and as I started to use Node.js v0.8, I was buzz with the path.existsSync being renamed. At the same time, I realized that some part of your module was using sync methods. So I took the liberty to update the code and I'm happy to send the changes back to you. Also, I've updated the test to mocha and should because some of your tests were achieving their goal (at a few places, you could negate the condition and no failure will be reported by expresso).
Thanks for this module, regards, d.

@wdavidw
Copy link
Author

wdavidw commented Jul 26, 2012

I'm must add that the readme reflect a new version 0.4.2 in case you accept the pull request and publish it to npm, package.json is not yet updated.

@slaskis
Copy link
Member

slaskis commented Aug 13, 2012

I already had the tests rewritten to mocha a while ago actually but never commited them as I started on a rewrite of the library which is more stream-focused but now I added some of your tests as well as 0.8 compatibility.

Really appreciated, thanks!

@slaskis slaskis closed this Aug 13, 2012
@wdavidw
Copy link
Author

wdavidw commented Aug 13, 2012

Thanks for your commitment to this project

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