Skip to content

Conversation

@timbod7
Copy link
Contributor

@timbod7 timbod7 commented Nov 11, 2015

I've included a simple script I used for testing.

It would be nice to have a test suite for this library

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it would be better handled by purescript-nullable. Then we wouldn't need to import two different versions using the FFI. Same for writeSeq. @garyb does that sound ok? Adding this as a dependency, I mean.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean so there's just read, and then we use toNullable on pos? Sounds good to me, that's one of the motivating use cases for it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that reminds me, we should be using ^0.2.0 of node-buffer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually don't worry about this, I'll take care of it later.

@hdgarrood
Copy link
Member

Great, thanks very much! I just have a couple of tweaks that I want to do before I release this.

hdgarrood added a commit that referenced this pull request Nov 12, 2015
@hdgarrood hdgarrood merged commit d975570 into purescript-node:master Nov 12, 2015
@timbod7
Copy link
Contributor Author

timbod7 commented Nov 12, 2015

I just have a couple of tweaks that I want to do before I release this.

Will this include adding the new Buffer effect type to the signature of the relevant functions in this package?

Yesterday I wrote code that looks like this:

do
      let buf = create size
      fdNext fd buf
      pure buf

which makes me unhappy.

@hdgarrood
Copy link
Member

Yes. Also, one of those tweaks is updating to node-buffer ^0.2.0, where create returns an Eff action instead of being a pure function (which is a lie).

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.

3 participants