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

@luislavena luislavena commented Aug 26, 2017

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
RX14
RX14 approved these changes Aug 27, 2017
@sdogruyol
Copy link
Member

@sdogruyol sdogruyol commented Aug 28, 2017

This looks good to go 👍 LGTM?

@ysbaddaden
Copy link
Contributor

@ysbaddaden ysbaddaden commented Aug 28, 2017

Thank you!

@ysbaddaden ysbaddaden merged commit fcce0c2 into crystal-lang:master Aug 28, 2017
2 checks passed
@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

@akzhan akzhan Aug 29, 2017

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

@bew bew Aug 29, 2017

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 Aug 29, 2017
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.

7 participants