-
Notifications
You must be signed in to change notification settings - Fork 57
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
Int to Bool conversion compatible with v0.4 #24
Conversation
Added "bonus" commit replacing constructor calls for conversions with explicit calls to |
Seems legit -- I haven't had a chance to run it but I'll try later and merge unless someone beats me to it |
@@ -71,17 +71,17 @@ function send_fragment(ws::WebSocket, islast::Bool, data::Array{Uint8}, opcode=0 | |||
b1::Uint8 = (islast ? 0b1000_0000 : 0b0000_0000) | opcode | |||
if l <= 125 | |||
write(ws.socket,b1) | |||
write(ws.socket,uint8(l)) | |||
write(ws.socket,convert(Uint8,l)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since Uint8
and friends are going away, spelling-wise, my preference is to bite the bullet and add Compat.jl as a dependency (@compat(UInt8(l))
). Most packages benefit from using Compat in more than one place/way, so 'bite the bullet' may be a little dramatic. This certainly works as is, though, if you want to use convert
for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hadn't heard about Uint8 and such going away. Can you point me to the
discussion? What's replacing it?
On Thu, Jul 2, 2015 at 5:20 PM, Sean Garborg [email protected]
wrote:
In src/WebSockets.jl
#24 (comment):@@ -71,17 +71,17 @@ function send_fragment(ws::WebSocket, islast::Bool, data::Array{Uint8}, opcode=0
b1::Uint8 = (islast ? 0b1000_0000 : 0b0000_0000) | opcode
if l <= 125
write(ws.socket,b1)
- write(ws.socket,uint8(l))
- write(ws.socket,convert(Uint8,l))
Since Uint8 and friends are going away, spelling-wise, my preference is
to bite the bullet and add Compat.jl as a dependency (@compat(UInt8(l))).
Most packages benefit from using Compat in more than one place/way, so
'bite the bullet' may be a little dramatic. This certainly works as is,
though, if you want to use convert for now.—
Reply to this email directly or view it on GitHub
https://github.com/JuliaWeb/WebSockets.jl/pull/24/files#r33788719.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uint
types are here for now, until question about type deprecation is resolved, julia-#11200.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UInt8
and friends replaced their lowercase-'i' predecessors in JuliaLang/julia#8907. Looks like JuliaLang/julia#11200 is holding up the deprecation of the lowercase versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we use Compat
then 0.4 style should be preferable, otherwise looks fine.
I'm sorry about how this went down, but these changes ended up merged to master via a few other PRs that introduced compat.jl and julia 0.4 syntax. Thanks for putting in the effort to do this though! |
Small change, but silences warnings on Julia v0.4.