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

Fix depwarns on julia 0.7 #492

Merged
merged 1 commit into from
Jul 25, 2018
Merged

Fix depwarns on julia 0.7 #492

merged 1 commit into from
Jul 25, 2018

Conversation

carlobaldassi
Copy link
Contributor

With this, tests pass (locally) without deprecation warnings on both 0.6 and 0.7.
It also fixes a warning about using Mmap, which doesn't appear in HDF5 tests but appears when running JLD tests.

It also enables CI testing on 0.7.

@carlobaldassi
Copy link
Contributor Author

Travis failure on OS X (all versions) is #483, unrelated to this PR.
The Appveyor failures on Win32 (julia 0.7) are also unrelated, it's hanging on the "build LibCURL" step.

@@ -1,6 +1,9 @@
using HDF5
using Compat.Test
using Compat.Distributed
@static if VERSION ≥ v"0.7.0-DEV.3637"
Copy link
Member

Choose a reason for hiding this comment

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

Is the @static is necessary here since it's in global scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I understand, @static means "evaluate the conditional at parse time". It shouldn't be related to the scope.

src/HDF5.jl Outdated
@@ -1969,7 +1982,7 @@ end
### Convenience wrappers ###
# These supply default values where possible
# See also the "special handling" section below
h5a_write(attr_id::Hid, mem_type_id::Hid, buf::String) = h5a_write(attr_id, mem_type_id, Vector{UInt8}(buf))
h5a_write(attr_id::Hid, mem_type_id::Hid, buf::String) = h5a_write(attr_id, mem_type_id, Vector{UInt8}(codeunits(buf)))
Copy link
Member

Choose a reason for hiding this comment

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

isn't it better to use unsafe_wrap here instead to avoid the extra allocation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

also enables CI testing on 0.7
@musm
Copy link
Member

musm commented Jul 25, 2018

thank you very much, will merge after latest tests are complete

@musm musm merged commit 236726c into JuliaIO:master Jul 25, 2018
@carlobaldassi carlobaldassi deleted the fix07 branch July 26, 2018 14:12
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

2 participants