search

bkaradzic / go-lz4

Port of LZ4 lossless compression algorithm to Go
BSD 2-Clause "Simplified" License
211 stars 23 forks source link

Incompatible NewWriter signature #2

Closed raichu closed 10 years ago

raichu commented 11 years ago

The typical signature for NewWriter is func NewWriter(wr io.Writer) *Writer whereas the LZ4 code has NewWriter(r io.Reader) io.ReadCloser. This makes it unusable within normal Go programs

raichu commented 11 years ago

Any updates on this issue?

bkaradzic commented 11 years ago

Nope. It's pretty low priority for me right now. If you fix it you can submit patch.

dgryski commented 10 years ago

Since we no longer have these API calls, this issue can probably be closed.

raichu commented 10 years ago

Since we no longer have these API calls

What do you mean specifically? I'm talking about Go standard library, and it still uses Reader/Writer interfaces extensively.

dgryski commented 10 years ago

I pushed a large change that removed the streaming implementation of lz4 encoding/decoding. This changed the API so that there we no longer returned a Writer or a Reader. Thus, this bug no longer applies.

A new bug might be"replace the streaming interface".

raichu commented 10 years ago

This recent change clearly kills off the selling point of this go package. C code, which is much faster than go code, already works in this encode/decode manner, and there are cgo wrappers for it around. Can't see any point of having such a package except for app engine users.

dgryski commented 10 years ago

Pure Go implementations still make deployment easier because they have no dependency on having the C library already installed. However, I see your point about removing major functionality. I was concerned about that when I submitted the pull request in the first place. Note you can always fork and save the commit just before the pull request was merged.

That being said, I'll look at what the C library provides in terms of a streaming API and what might make sense to add to this implementation. Or even submit patches to the cgo bindings to make sure they expose that functionality.