BioJulia / Libz.jl

Fast, flexible zlib bindings.

Other

27 stars 17 forks source link

Create Libz, Gzip, and Raw stream types #51

Closed Hydrotoast closed 5 years ago

Hydrotoast commented 7 years ago

Pull request for issue #50. Replaces the ZlibXYStream API with:

{Zlib, Gzip, Raw}{Inflate, Deflate}{Input, Output}Stream

The three formats Zlib, Gzip, and Raw are the different wrapper types that zlib supports.

codecov-io commented 7 years ago

Codecov Report

Merging #51 into master will increase coverage by 0.75%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
+ Coverage   85.11%   85.87%   +0.75%     
==========================================
  Files           6        6              
  Lines         262      269       +7     
==========================================
+ Hits          223      231       +8     
+ Misses         39       38       -1

Impacted Files	Coverage Δ
src/Libz.jl	`100% <ø> (ø)`	:arrow_up:
src/source.jl	`89.18% <100%> (+1.68%)`	:arrow_up:
src/sink.jl	`87.95% <100%> (+0.77%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b9f5ed5...27ca768. Read the comment docs.

bicycle1885 commented 7 years ago

Thank you @Hydrotoast. I'll take a look today.

Hydrotoast commented 7 years ago

Since this is primarily an API change, the owner should tag the commit with a :major version after merging. This was recommended by the PkgDev README.

> using PkgDev
> PkgDev.tag(:major)
> PkgDev.publish()

Hydrotoast commented 7 years ago

@bicycle1885 friendly review reminder :)

bicycle1885 commented 7 years ago

I'm sorry for being late! i've added a comment.

To be honest, I don't think it is a good idea to change the behavior of ZlibInflateInputStream. It will break some programs depending on that behavior. This package is very commonly used in the Julia community (16,500 downloads per two weeks), and hence I think it may result in the frustration of many developers. Other changes look good to me.

Hydrotoast commented 5 years ago

Thanks for the reviews @bicycle1885 !

Sorry to bring up this old PR again, but I am closing it to keep the PR list tidy. :)