-
-
Notifications
You must be signed in to change notification settings - Fork 359
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
replace build.jl by Artifacts.toml #3023
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3023 +/- ##
==========================================
- Coverage 62.27% 62.27% -0.01%
==========================================
Files 25 25
Lines 6397 6399 +2
==========================================
+ Hits 3984 3985 +1
- Misses 2413 2414 +1
Continue to review full report at Codecov.
|
What exactly should be tested? If plotly works when building with this branch? |
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.
Works for me, also with ENV["PLOTS_HOST_DEPENDENCY_LOCAL"] = "true"
I don't really know when this environment variable is actually used... I guess it might be an IJulia thing. Building shouldn't be necessary anymore. |
Yes, works for me also in IJulia with |
Great, thanks for checking! |
nice! |
I am pretty sure this does not work; artifact downloads has to be .tar.gz? |
use_local_dependencies[] = isfile(plotly_local_file_path) && use_local_plotlyjs[] | ||
if use_local_plotlyjs[] && !isfile(plotly_local_file_path) | ||
@warn("PLOTS_HOST_DEPENDENCY_LOCAL is set to true, but no local plotly file found. run Pkg.build(\"Plots\") and make sure PLOTS_HOST_DEPENDENCY_LOCAL is set to true") | ||
if get(ENV, "PLOTS_HOST_DEPENDENCY_LOCAL", "false") == true |
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 path is never taken, since what you get back is a the string "true"
if you have set it to that, so that is never == true
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 changed that in #3067
From JuliaLang/Pkg.jl#1921 and the iris exampe I got the impression it does work this way, if I don't use the artifact string macro |
I'm still a bit shaky if that really works out. So if someone could test this branch, who uses this feature, I would be happy.
Fix: #1783