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

Normalize Flate's block interface to .open #4887

Merged
merged 1 commit into from Aug 28, 2017
Merged

Normalize Flate's block interface to .open #4887

merged 1 commit into from Aug 28, 2017

Conversation

luislavena
Copy link
Contributor

Adjust both Flate::Reader and Flate::Writer interfaces for block method .open, offering auto-close of reader/writer instance, respectively.

This follows the interface defined by Gzip::Reader and Gzip::Writer, ensuring a uniform API for these compression methods.

Add specs to ensure these block methods are covered and adjust documentation to highlight io parameter.

Closes #4630

Thank you ❤️ ❤️ ❤️

Adjust both `Flate::Reader` and `Flate::Writer` interfaces for block
method `.open`, offering auto-close of reader/writer instance, respectively.

This follows the interface defined by `Gzip::Reader` and `Gzip::Writer`,
ensuring a uniform API for these compression methods.

Add specs to ensure these block methods are covered and adjust documentation
to highlight `io` parameter.

Closes #4630
@sdogruyol
Copy link
Member

This looks good to go 👍 LGTM?

@ysbaddaden
Copy link
Contributor

Thank you!

@ysbaddaden ysbaddaden merged commit fcce0c2 into crystal-lang:master Aug 28, 2017
@mverzilli mverzilli added this to the Next milestone Aug 28, 2017
reader = new input, sync_close: sync_close, dict: dict
# Creates a new reader from the given *io*, yields it to the given block,
# and closes it at its end.
def self.open(io : IO, sync_close : Bool = false, dict : Bytes? = nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just mention that signature looks better with:

def self.open(io : IO, sync_close : Bool = false, dict : Bytes? = nil) : Nil

It's fine for compiler and fine for documentation.

Copy link
Contributor

Choose a reason for hiding this comment

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

just to mention (nested mention ^^), sync_close : Bool = false is quite redundant to specify the type.. I think this could be avoided without downside on the documentation.

@luislavena luislavena deleted the normalize-flate-api branch August 29, 2017 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC: Flate Reader/Writer API inconsistency compared with Gzip
7 participants