Conversation
rofinn
left a comment
rofinn
left a comment
There was a problem hiding this comment.
LGTM, just a couple suggestions you might want to consider.
| @@ -0,0 +1,64 @@ | |||
| name: CI | |||
| on: | |||
| - push | |||
There was a problem hiding this comment.
Might want to limit this to
push:
branches: [master]
tags: ["*"]
| version: | ||
| - '1.0' | ||
| - '1' # automatically expands to the latest stable 1.x release of Julia | ||
| - 'nightly' |
There was a problem hiding this comment.
This might be noisy as there isn't an equivalent to allow failures.
There was a problem hiding this comment.
We could have it in a separate step which is allowed to fail, like JuliaStats/StatsModels.jl#199.
There was a problem hiding this comment.
FWIW the statsmodels setup doesn't actually allow it to fail in the strict sense that you'll still get the red X on the commit if it's failing. but you can at least see isolate it in a separate workflow so you can have the nice green badge.
There was a problem hiding this comment.
Yeah, I guess if you aren't editing these files that often that's probably fine.
| arch: | ||
| - x64 | ||
| - x86 | ||
| exclude: |
There was a problem hiding this comment.
Do we actually need to test on all combinations of julia version, os and architecture?
There was a problem hiding this comment.
Yeah that's a difficult point. AFAIK we used to do that until now. Another solution would be to do something like JuliaLang/Example.jl#55. But given that StatsBase has a relatively low activity it may be OK to run many checks.
There was a problem hiding this comment.
Yeah, that's what we do. If you don't think StatsBase is gonna be that active going forward then I'm fine with leaving it.
|
OK let's go with this, we can refine it later but at least we'll have CI again. |
3 participants
No description provided.