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

Refactor the deflate, gz and zlib modules & rename types #120

Merged
merged 2 commits into from
Aug 30, 2017
Merged

Refactor the deflate, gz and zlib modules & rename types #120

merged 2 commits into from
Aug 30, 2017

Conversation

sbstp
Copy link
Contributor

@sbstp sbstp commented Aug 28, 2017

I've re-factored the modules and types according to what was suggested in #75. Using pub(crate) in a few places, I was able to avoid having to create multiple layers of modules. Here's a summary of the changes:

  • Split deflate.rs, gz.rs and zlib.rs into multiple files. Each module has a mod.rs, bufread.rs, read.rs and write.rs.
  • The structs were renamed properly. EncoderWriter becomes write::(Deflate|Gz|Zlib)Encoder. DecoderReader becomes read::(Deflate|Gz|Zlib)Decoder. This was done for all the combinations.
  • The *Buf structs are moved to their respective bufread submodule.
  • The *Reader structs are moved to their respective read submodule.
  • The *Writer structs are moved to their respective write submodule.
  • Sprinkled a few pub(crates) here and there to keep the API unchanged.
  • Grouped the struct declarations with their impl blocks in each file.
  • The unit tests were moved to their respective mod.rs.

@sbstp
Copy link
Contributor Author

sbstp commented Aug 28, 2017

I can see in the travis log that I forgot to cleanup the imports when different features are used. On it.

@alexcrichton
Copy link
Member

Thanks so much for this! And wow that's quite the diff :)

I'll poke around with this and probably merge promptly after!

@alexcrichton alexcrichton merged commit e8a11e7 into rust-lang:master Aug 30, 2017
@sbstp sbstp deleted the refactor branch August 30, 2017 17:46
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