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
Merged

Conversation

AndyGauge
Copy link
Contributor

@AndyGauge 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
Copy link
Member

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

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

…xamples to be compiled into documentation. Examples are for Zlib and Gzip structs
@AndyGauge
Copy link
Contributor Author

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

@alexcrichton
Copy link
Member

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

@AndyGauge
Copy link
Contributor Author

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
Copy link
Member

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

@alexcrichton alexcrichton merged commit a73c33e into rust-lang:master May 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants