-
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
Change CI to test julia 1.0 instead of julia 0.6 #17
Conversation
Reopened to rerun travis |
.travis.yml
Outdated
@@ -13,9 +13,6 @@ git: | |||
depth: 99999999 | |||
before_install: | |||
- if [[ -a .git/shallow ]]; then git fetch --unshallow; fi |
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 think this (and the git depth option) are unnecessary now? If not, it appears we can use depth: false
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.
Yep, it looks like those are in the default now. I'll remove them.
.travis.yml
Outdated
@@ -13,9 +13,6 @@ git: | |||
depth: 99999999 | |||
before_install: | |||
- if [[ -a .git/shallow ]]; then git fetch --unshallow; fi | |||
script: | |||
- julia -e 'Pkg.clone(pwd()); | |||
Pkg.test("Glob", coverage=true)' | |||
after_success: | |||
- julia -e 'cd(Pkg.dir("Glob")); |
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.
Seems to be failing? I'm not sure how to do this now, given that Pkg.dir
is deprecated.
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.
┌ Warning:
Pkg.dir(pkgname, paths...)
is deprecated; instead, doimport Glob; joinpath(dirname(pathof(Glob)), "..", paths...)
.
└ @ Pkg.API /Users/osx/buildbot/slave/package_osx64/build/usr/share/julia/stdlib/v0.7/Pkg/src/API.jl:454
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.
But that's terrible. So I also opened travis-ci/travis-ci#9982 to make this the default behavior.
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.
Also probably wrong, since we don't want to do import Glob
in this process. @StefanKarpinski how do we correctly get the path to something without loading it with the new loading.jl
code?
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.
Oh, actually, I guess the issue is that the old Pkg code needed to make a copy of this code. In the new Pkg manager, this line should be unnecessary?
PTAL: Okay cool, this passes travis now. |
Actually, one more thing, should we bump the REQUIRES version to 0.7 since we removed tests for 0.6 here? |
.travis.yml
Outdated
- julia -e 'cd(Pkg.dir("Glob")); | ||
- julia -e 'using Pkg; | ||
import Glob; | ||
cd(dirname(dirname(pathof(Glob)))); |
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.
we don't want to import Glob into this process (the deprecation message is wrong), but I think dirname(dirname(pathof(Glob))))
is supposed to be literally pwd()
?
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.
Oh, good point, yeah i think you're right... haha sorry. :[
Yeah that should do just fine. 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.
Yep, and we just checked that it works over at Pidfile.jl (copying your work here 🙂)
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.
😄 Cool, sounds good! Thanks
Also, yes, I think we should bump REQUIRES |
Okay great. Done! :) This should be good to merge whenever! :) |
(Okay, actually, i also went ahead and removed all the |
src/Glob.jl
Outdated
|
||
import Base: ismatch, readdir, show | ||
import Compat: occursin | ||
import Base: ismatch, readdir, show, occursin |
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.
WARNING: could not import Base.ismatch into Glob
Everything else looks good to me.
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.
Fixed, thanks! :)
src/Glob.jl
Outdated
|
||
import Base: ismatch, readdir, show | ||
import Compat: occursin | ||
import Base: ismatch, readdir, show, occursin | ||
|
||
@static if VERSION < v"0.7.0-DEV.5126" |
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.
can drop this code 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.
Ah good catch. There were actually a bunch of these, and I just removed them all. :)
No description provided.