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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove sync version? #2

Closed
lukeed opened this issue Feb 19, 2018 · 2 comments
Closed

Remove sync version? #2

lukeed opened this issue Feb 19, 2018 · 2 comments
Labels
enhancement New feature or request

Comments

@lukeed
Copy link
Contributor

lukeed commented Feb 19, 2018

This looks almost exactly like one of my internal tools 馃槅

I would maybe suggest that since you're shipping code with async syntax, that there's no/little point in shipping a sync version of the code.

This is because having async (and fs.rmdir) in the code at all, even when not imported, forces a Node 7.6+ environment, wherein native async is available.

So, anyone using this lib can/should have an async parent function that awaits this guy. You'd only be dropping support for top-level usage, which is easily fixed anyway.

@terkelg terkelg added the enhancement New feature or request label Feb 19, 2018
@terkelg
Copy link
Owner

terkelg commented Feb 19, 2018

Agree fd44c02 removes the sync version. 馃憤

@lukeed
Copy link
Contributor Author

lukeed commented Feb 19, 2018

Nice~! 馃帀

@lukeed lukeed closed this as completed Feb 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants