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

readdir should take absolute path for uniformity with other methods #19

Closed
cjdell opened this issue Oct 20, 2015 · 12 comments
Closed

readdir should take absolute path for uniformity with other methods #19

cjdell opened this issue Oct 20, 2015 · 12 comments

Comments

@cjdell
Copy link
Collaborator

cjdell commented Oct 20, 2015

@johanneslumpe Do you think it is reasonable to drop the 2nd argument and make readDir take an absolute path? It's not an issue that can't be worked around but I think new comers might think this is a little odd?

I'm raising this now so we don't get design quirks set in stone when we reach version 1.0 :-)

@johanneslumpe
Copy link
Collaborator

@cjdell Yeah I think that would be fine. For that we then definitely have to bump to 1.0 before shipping. But that's cool with me :)

@cjdell
Copy link
Collaborator Author

cjdell commented Oct 21, 2015

@johanneslumpe Awesome, it that case might also make readFile and writeFile more uniform and more Node like (basically add encoding param). :-)

@cjdell
Copy link
Collaborator Author

cjdell commented Oct 22, 2015

@johanneslumpe I'm very happy with the API now, it is now more Node like. I think a feature freeze might be in order.

By the way I appreciate you giving me the freedom to make these changes. I really hope this becomes a popular library. :-)

@johanneslumpe
Copy link
Collaborator

@cjdell I don't think the features of the lib ever changed since I published it :D

@cjdell
Copy link
Collaborator Author

cjdell commented Oct 22, 2015

@johanneslumpe Well I added a downloadFile to bypass JS pipeline because writeFile is slow. Other than that it has mostly been interface changes and of course Android support.

Is it easy enough to publish the module? Do you want to commit to a 1.0 release?

@johanneslumpe
Copy link
Collaborator

@cjdell Oh I didn't notice that method before! Cool - did you test it thoroughly? Also in terms of connection errors etc?

Publishing this is easy yeah :) We can do a 1.0 release.

@cjdell
Copy link
Collaborator Author

cjdell commented Oct 23, 2015

@johanneslumpe Made some minor fixes today. I've noticed in Android that downloadFile blocks the UI on large files but not in iOS. Will need to have some fun with threads. Will have to be tomorrows job. :-)

@johanneslumpe
Copy link
Collaborator

@cjdell oh that's a bummer :( I hope you can figure that out though! 👍

@johanneslumpe
Copy link
Collaborator

@cjdell Is everything working as expected now? I saw you changed the downloadFile method to an AsyncTask

@cjdell
Copy link
Collaborator Author

cjdell commented Oct 29, 2015

@johanneslumpe Yeah it's working much better now. Nearly ready to publish my Android app. You might as well publish to NPM :-)

@johanneslumpe
Copy link
Collaborator

@cjdell
Copy link
Collaborator Author

cjdell commented Nov 2, 2015

@johanneslumpe Awesome!

@cjdell cjdell closed this as completed Nov 2, 2015
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

No branches or pull requests

2 participants