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

Allow custom fs adapter to be passed as option (#280) #285

Closed
wants to merge 1 commit into from

Conversation

erikkemperman
Copy link
Contributor

Maybe I should have allowed more time for negative feedback, but eager to get on with my planned refactor in vinyl-fs I figured I might as well take a stab at proposing something concrete.

@erikkemperman
Copy link
Contributor Author

Maybe point out the obvious: I haven't considered changes for the sync case yet. Also, it just occurred to me I've just assumed custom adapters would support cache argument to realpath but probably shouldn't.

@erikkemperman
Copy link
Contributor Author

erikkemperman commented Sep 7, 2016

I've added the synchronous counterpart, and tentatively simplified to pre-empt realpath calls entirely when a custom fs option is provided. Underestimated the implications of Node 6 slightly there.

Will leave this as it is for now, until you get a chance to look at it (or decide it's a bad idea). Apologies for having submitted this a bit prematurely!

@erikkemperman
Copy link
Contributor Author

Rebased onto latest master.

@erikkemperman
Copy link
Contributor Author

Rebased again. Noticed failing test on Appveyor, have submitted a separate PR which I believe addresses that.

@isaacs I appreciate you must be quite busy, but if possible I would be grateful for any quick feedback you might have here -- even if it's just that you don't like this idea... Thanks!

@isaacs
Copy link
Owner

isaacs commented Mar 1, 2023

This will have to be added as an option to https://github.com/isaacs/path-scurry. If so, it'll be trivial to pass it along from glob.

@isaacs isaacs closed this Mar 1, 2023
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

2 participants