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

Better migration fixes for path.existsSync #21

Open
shama opened this issue Jul 7, 2012 · 4 comments
Open

Better migration fixes for path.existsSync #21

shama opened this issue Jul 7, 2012 · 4 comments

Comments

@shama
Copy link

shama commented Jul 7, 2012

Take a look at this pull: https://github.com/cowboy/node-glob-whatev/pull/3/files Both changes are incorrect.

I think a better migration would be something a la var existsSync = fs.existsSync || path.existsSync; so it doesn't break code on 0.6. Thanks!

@shama
Copy link
Author

shama commented Jul 7, 2012

Oh another note: it misses uses of require('path').existsSync too. :)
Reference: gruntjs/grunt-contrib#73 (btw, has been fixed earlier in the 0.2.0-wip branch of that repo)

@AndreasMadsen
Copy link

+1 just got 5 pull requests, there will break compatibility with node 0.6 and node 0.4.

@bithavoc
Copy link

bithavoc commented Jul 7, 2012

+1

@blakmatrix
Copy link
Owner

I'm kinda sad because the state of the bot is relatively ephemeral, it takes only hours to run on npm repos on github (this time around 8672 and only affected 8% of the population(700) of those 60% were positive changes and 40% were not) however, when i package this into an example, I will attempt to address the require('fs') and backwards compat both ways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants