-
Notifications
You must be signed in to change notification settings - Fork 18
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 various depwarns under 0.7 #21
Conversation
@@ -49,15 +49,15 @@ using Compat | |||
end | |||
|
|||
if VERSION >= v"0.5.0-" | |||
immutable ASCIIString <: DirectIndexString | |||
struct ASCIIString <: DirectIndexString |
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.
This package still supports 0.4 but struct
only parses properly on 0.6+.
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.
Good catch! I should change the version requirements as well.
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.
Bumped REQUIRE and CI to Julia 0.6+. Thanks!
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.
Presumably this package will be mostly unnecessary as packages move away from 0.4 support, so bumping the requirement to 0.6 might not be worth it at this point. Probably no harm in doing so though. Seems like a good question for @tkelman.
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.
Does tagging a release prior to this pull request and then carrying on do the trick? Or does that approach present pitfalls?
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.
+1 for simply making this package unnecessary
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.
That sounds like the best approach insofar as possible! :)
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.
(Looks like JLDArchives, StringEncodings, JLD, SALSA, and SQLite depend on this package.)
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.
SALSA.jl uses UTF32String
for some reason, I can't see why, since it is not for interop.
SQLite.jl uses UTF16String
for calling external C interfaces, but it looks like it could use Base.transcode
instead.
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.
StringEncodings uses LegacyStrings mainly as a way exercize support for string types in different encodings. I could easily remove it if LegacyStrings is declared obsolete.
OTOH, if that's not too much work keeping the package alive makes sense, as these string types can be useful for specialized use cases. Of course somebody could revive it if it's really needed.
This was superseded by #29 and can be closed. |
See title. Best! :)