search

reasonml-old / bs-node

Node bindings for BuckleScript
78 stars 17 forks source link

Node.Fs.Watch API #11

Open gilbert opened 6 years ago

gilbert commented 6 years ago

Is this API incomplete? The binding returns a NodeStringBuffer, but that module has no methods in it.

ncthbrt commented 6 years ago

@gilbert are you referring to the built in Node module? That is distinct from this one. This repo was forked a while ago from the built in Node module. This API does have some methods that the built in one does not, but no one has really made a concerted effort to work on this library for a while.

I'm of the opinion that efforts towards covering the node API surface would be better if they were broken into mutually compatible, smaller parts. If you'd like I could create a bs-node-fs repo in reasonml-communnity for example and if you'd like to add bindings, we could add them there?

gilbert commented 6 years ago

Yeah I did notice that this repo has more methods covered than Belt's Node module. Are the changes here expected to eventually be merged into Belt?

Given there's only a small amount of community effort, I think keeping things centralized as it is now will make things easier to maintain, also considering how Node's API has many cross-module concerns.

If you're willing, I would be happy to become a core contributor to this repo. I'm currently doing a large amount of backend work for Node + ReasonML (streams, file i/o, http, etc), and would like to give back by pushing up any required new bindings / changes. It would also be great to merge in @lightsofapollo's significant work from bs-node-ext.

ncthbrt commented 6 years ago

Yes. That would be fantastic @gilbert

I am the designated maintainer, happy to still do the leg work of responding to issues, reviewing PRs, etc, but have a bit too much on my plate maintaining other OSS stuff as well, so haven't been able to commit much time to actually populating this repo.

Original idea was to merge this stuff upstream, but we could for now publish it separately while it gains maturity.

siffogh commented 5 years ago

Was there a repo created? I wanted to add some bindings as well for fs.readDir to accept options. If no repo was created, I would love to do it and add some bindings.