added compress options for gzip #7730

Merged
merged 1 commit into from Feb 10, 2013

4 participants

@beyond

Zlib::GzipWriter can take two optional arguments, compression level and strategy. This fix is to pass these arguments.

http://www.ensta-paristech.fr/~diam/ruby/online/ruby-doc-stdlib/libdoc/zlib/rdoc/classes/Zlib/GzipWriter.html

@steveklabnik
Ruby on Rails member

Do you think there's some way that a test could be written for this?

@beyond

I have no good idea for testing wrapper object. If I could have a testing method, I write a code.

@beyond

I wrote some test codes for gzip options.

@frodsan frodsan commented on the diff Oct 26, 2012
activesupport/lib/active_support/gzip.rb
@@ -25,9 +25,9 @@ def self.decompress(source)
end
# Compresses a string using gzip.
- def self.compress(source)
@frodsan
frodsan added a note Oct 26, 2012

Please, can you add docs about the new options? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@steveklabnik
Ruby on Rails member

/cc @pixeltrix @fxn, what do you guys think about this?

@fxn
Ruby on Rails member
fxn commented Dec 3, 2012

Looks good to me, the signature of the method shown by RDoc will be enough documentation for those parameters I believe, since they match gzip jargon and are self-explanatory.

We would need a CHANGELOG entry though.

@pixeltrix
Ruby on Rails member

Ditto to what @fxn said - also squash the commits please.

@fxn
Ruby on Rails member
fxn commented Dec 8, 2012

Hi @beyond a CHANGELOG entry and commits squash is all we need here, if you find a minute we'll apply.

@beyond

Hi @fxn I wrote a CHANGELOG and squashed commits.

@fxn
Ruby on Rails member
fxn commented Feb 8, 2013

Hi @beyond, could you please revise this pull request? I tried pulling the branch to do the merge myself, but some changes related to engines come down for some reason.

@beyond beyond added compress options for gzip
added test for compress options of gzip

update changelog
d59a877
@beyond

Hi @fxn This branch rebased the master branch and fixed conflicts.

@fxn
Ruby on Rails member

Looking good, thanks very much!

@fxn fxn merged commit 95dd699 into rails:master Feb 10, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment