Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added compression examples #111

Merged
merged 1 commit into from May 12, 2017

Conversation

Projects
None yet
2 participants
@AndyGauge
Copy link
Contributor

AndyGauge commented May 9, 2017

For #76
NOT FINISHED
Before completing the decompression examples, and possibly adding other methods to the examples, I was hoping for a code review to confirm these are desired.

cargo run --example zlibencoder-bufread
cargo doc
@alexcrichton

This comment has been minimized.

Copy link
Owner

alexcrichton commented May 10, 2017

Awesome thanks so much for doing this! I'll take a look now

@alexcrichton
Copy link
Owner

alexcrichton left a comment

Ok other than a few minor things looks great!

Could you also throw a comment at the top of each example with just a small word or two as to what it's doing at a high level?

println!("{}", decode_bufreader(bytes).unwrap());
}

fn decode_bufreader(bytes: Vec<u8>) -> Result<String, std::io::Error> {

This comment has been minimized.

@alexcrichton

alexcrichton May 10, 2017

Owner

The return type here may be a bit more readable as io::Result<String> perhaps? That tends to be how Result<T, io::Error> is referenced idiomatically

}

fn decode_bufreader(bytes: Vec<u8>) -> Result<String, std::io::Error> {
let cursor = Cursor::new(bytes);

This comment has been minimized.

@alexcrichton

alexcrichton May 10, 2017

Owner

I think you can actually elide the Cursor here via impl Read for &[u8], it'll just involve transforming bytes: Vec<u8> to bytes: &[u8] and then passing that into the decoder.

Examples directory provides runnable examples and doc comments have e…
…xamples to be compiled into documentation. Examples are for Zlib and Gzip structs

@AndyGauge AndyGauge force-pushed the AndyGauge:examples branch from b27d032 to 4f949ba May 11, 2017

@AndyGauge

This comment has been minimized.

Copy link
Contributor Author

AndyGauge commented May 11, 2017

I rebased this, I didn't like the number of commits in the PR.

@alexcrichton

This comment has been minimized.

Copy link
Owner

alexcrichton commented May 11, 2017

No worries! Is this ready to go? It's looking absolutely fantastic!

@AndyGauge

This comment has been minimized.

Copy link
Contributor Author

AndyGauge commented May 11, 2017

I'm at a stopping point. I'll work on Deflate next. Feel free to merge this and I'll open up another PR for Deflate.

@alexcrichton

This comment has been minimized.

Copy link
Owner

alexcrichton commented May 12, 2017

Sounds great to me, thanks so much again @AndyGauge!

@alexcrichton alexcrichton merged commit a73c33e into alexcrichton:master May 12, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.