You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@commons.apache.org by cgrushko <gi...@git.apache.org> on 2018/07/24 07:38:51 UTC
[GitHub] commons-compress pull request #68: Allow specifying Zstandard compression le...
GitHub user cgrushko opened a pull request:
https://github.com/apache/commons-compress/pull/68
Allow specifying Zstandard compression level
ZstdCompressorOutputStream allows data compression using Zstandard, but the default compression level of 3 is hard-coded.
Programs that wish to use a different compression level must reimplement ZstdCompressorOutputStream.
This PR adds a constructor that takes a compression level and passes it to
`new ZstdOutputStream(...)`.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cgrushko/commons-compress master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/commons-compress/pull/68.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #68
----
commit 06aaeda033109060ef2d505ce09d7e7b27ef75f6
Author: Carmi Grushko <ca...@...>
Date: 2018-07-24T07:30:20Z
Allow specifying Zstandard compression level
ZstdCompressorOutputStream allows to compress data using Zstandard, but the default compression level of 3 is hard-coded.
Programs that wish to use a different compression level must reimplement ZstdCompressorOutputStream.
I suggest we add a constructor that takes a compression level and passes it to
`new ZstdOutputStream(...)`.
----
---
[GitHub] commons-compress issue #68: Allow specifying Zstandard compression level
Posted by bodewig <gi...@git.apache.org>.
Github user bodewig commented on the issue:
https://github.com/apache/commons-compress/pull/68
Well, you could still use zstd-jni directly instead of Commons Compress for the zstd part.
Many thanks for the PR.
---
[GitHub] commons-compress issue #68: Allow specifying Zstandard compression level
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/commons-compress/pull/68
[](https://coveralls.io/builds/18130986)
Coverage increased (+0.1%) to 86.698% when pulling **06aaeda033109060ef2d505ce09d7e7b27ef75f6 on cgrushko:master** into **6f8c4960c2af25b7e4b36473d81fe652490dbbeb on apache:master**.
---
[GitHub] commons-compress issue #68: Allow specifying Zstandard compression level
Posted by bodewig <gi...@git.apache.org>.
Github user bodewig commented on the issue:
https://github.com/apache/commons-compress/pull/68
Thank you @cgrushko
could you please add javadocs that explain the parameters (and a 1.18 @since marker would be a bonus).
---
[GitHub] commons-compress issue #68: Allow specifying Zstandard compression level
Posted by bodewig <gi...@git.apache.org>.
Github user bodewig commented on the issue:
https://github.com/apache/commons-compress/pull/68
Understood. I somehow assumed you'd know what the parameters did :-) I'll merge the PR as is.
In a way I wonder why anybody who needs that much control over zstd output is using Commons Compress as indirection at all.
---
[GitHub] commons-compress pull request #68: Allow specifying Zstandard compression le...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/commons-compress/pull/68
---
[GitHub] commons-compress issue #68: Allow specifying Zstandard compression level
Posted by cgrushko <gi...@git.apache.org>.
Github user cgrushko commented on the issue:
https://github.com/apache/commons-compress/pull/68
heh :)
Commons compress provides tar support which is almost a must for using
zstd.
Thanks!
On Sat, Jul 28, 2018 at 5:07 PM Stefan Bodewig <no...@github.com>
wrote:
> Understood. I somehow assumed you'd know what the parameters did :-) I'll
> merge the PR as is.
>
> In a way I wonder why anybody who needs that much control over zstd output
> is using Commons Compress as indirection at all.
>
> —
> You are receiving this because you were mentioned.
>
>
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-compress/pull/68#issuecomment-408610256>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AB5_YZuKz4qhMJoKHeq-0R6K4HCV3NVKks5uLHAfgaJpZM4VcPBY>
> .
>
---
[GitHub] commons-compress issue #68: Allow specifying Zstandard compression level
Posted by cgrushko <gi...@git.apache.org>.
Github user cgrushko commented on the issue:
https://github.com/apache/commons-compress/pull/68
@bodewig the ctors I'm calling don't have javadoc either, so I don't know what the parameters mean. I can add `@param level The compression level` on those that have a level, but I don't think it'll be helpful.
I can also add something along the lines of `Constructs a stream using ZstdOutputStream's ctor with the same prototype`, but that seems meh as well.
The class's purpose is to bridge two interfaces, it doesn't add anything on its own, so it's hard to document it beyond "go look at the other class we're delegating to".
(the original code didn't have any comments at all, for this reason, in my opinion)
If you have a specific javadoc in mind I'm happy to add it.
---