-
Notifications
You must be signed in to change notification settings - Fork 2
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
use artifacts from NCBI directly #4
use artifacts from NCBI directly #4
Conversation
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 agree this is nicer than using Conda - thanks for putting in the effort!
Just need to address the artifacts version thing - since it's a stdlib, I actually don't think it needs compat, but if you keep it, you need to relax the constraint a bit.
[[ncbi-blast]] | ||
arch = "x86_64" | ||
git-tree-sha1 = "690abc170201bc029d0e7386da0791d7a76d98ff" | ||
os = "macos" |
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.
Is this valid TOML? looks like it would overwrite the entries above...
Though I assume this was auto-generated by Artifacts.jl, so maybe I'm misunderstanding
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.
Yeah, it is not obvious, but the double brackets is toml syntax for "array of tables" https://toml.io/en/v1.0.0#array-of-tables
As discussed on Slack, using a JLL is still better, but I'd still accept this as an incremental improvement over Conda |
What system are you on @kescobo, would be nice to test on windows? Maybe the CI supports that? |
There is definitely a way to add Windows to CI, might have to find another package to copy from. I use Linux myself, but agree it would be nice to check |
I added some options to the github workflow, but don't know if I got it right. In any case, it looks like you need to approve running the workflows. |
Someone on slack reported an error on Windows (https://julialang.slack.com/archives/CAKKFNYLD/p1728896761497669?thread_ts=1728401460.511029&cid=CAKKFNYLD if you get in before slack eats it), but they were getting an error with the Conda version too, and said it might be their work firewall settings. Given that windows CI here passes, I'm gonna merge and just see what comes of it. |
add Artifact.toml generating script and Artifact.toml for blast 2.16.0