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

Change Bytes to ByteString #230

Merged
merged 2 commits into from
May 26, 2017
Merged

Conversation

erikd
Copy link
Contributor

@erikd erikd commented May 4, 2017

Bytes was a type alias for ByteString. The Bytes type is kept
because it is exposed in the API.

Have built http-client-tls and warp-tls from Hackage with this
version of tls and zero changes were needed in those packages.

Related to #229 . I have no problem whatsoever moving from Bytes/ByteString to ByteArray but while Bytes and ByteString are the same, Bytes should be replaced with ByteString if it can be done without breaking the API (and it can).

@erikd erikd force-pushed the topic/Bytes-API branch 2 times, most recently from 0790e04 to 8e17544 Compare May 5, 2017 06:59
@erikd
Copy link
Contributor Author

erikd commented May 5, 2017

The build failures are when building the tasty package. See UnkindPartition/tasty#166

@erikd erikd force-pushed the topic/Bytes-API branch 3 times, most recently from 1ce9935 to f97e95c Compare May 5, 2017 15:53
@erikd erikd mentioned this pull request May 6, 2017
`Bytes` was a type alias for `ByteString`. The `Bytes` type is kept
because it is exposed in the API.

Have built http-client-tls and warp-tls from Hackage with this
version of tls and zero changes were needed.
@erikd
Copy link
Contributor Author

erikd commented May 19, 2017

Ping?

@ocheron
Copy link
Contributor

ocheron commented May 19, 2017

There is still the question to deprecate or simply remove the type alias.
Since we couldn't decide, the safe way would be to deprecate in this PR.

@erikd
Copy link
Contributor Author

erikd commented May 19, 2017

The code in this PR will not cause any difficulty at all in client code. No compile errors, no warnings, no changes in behavior. I would therefore advise against deprecating the Bytes type until there is a more major change is required.

@erikd
Copy link
Contributor Author

erikd commented May 19, 2017

Since what is on master already has breaking changes, I'd be happy to remove the Bytes type all together or at the very least deprecating it.

@kazu-yamamoto
Copy link
Collaborator

We should ping @vincenthz.

@ocheron
Copy link
Contributor

ocheron commented May 26, 2017

I think we can't be wrong deprecating the type alias (and possibly moving the definition to Network.TLS itself). If it's used out of tls, it's probably by accident.

@erikd
Copy link
Contributor Author

erikd commented May 26, 2017

@ocheron I added a commit to do as you suggested.

@kazu-yamamoto
Copy link
Collaborator

Though @vincenthz does not respond, I would like to merge this PR.

@vincenthz vincenthz merged commit e08541e into haskell-tls:master May 26, 2017
@erikd erikd deleted the topic/Bytes-API branch May 26, 2017 22:23
ocheron added a commit that referenced this pull request May 29, 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.

None yet

4 participants