Support for weeder step#742
Conversation
phadej
left a comment
There was a problem hiding this comment.
Before I even look at this PR more closely. It contains independent changes like ghExprWrap and changes elsewhere. Do them separately. I don't know whether I like them or not.
Ok, I have removed the independent changes from this branch. |
| let ifCond = ghCompilerVersionArithPredicate allVersions range in | ||
| for_ pkgs $ \Pkg{pkgName} -> do | ||
| githubUsesIf "weeder" "freckle/weeder-action@v2" ifCond $ buildList $ do | ||
| item ("ghc-version", ghWrapExpr "matrix.compilerVersion") |
There was a problem hiding this comment.
I don't see this working. Comparing strings like 8.10.7 and 8.8.4 with operators like >= wouldn't work as intended.
Add test
There was a problem hiding this comment.
Comparing strings like
8.10.7and8.8.4with operators like>=wouldn't work as intended.
The strings get converted to numeric representation before comparison. E.g., with this config:
weeder: >= 9.4 && < 9.8
the resulting comparison is:
${{ env.HCNUMVER >= 90400 && env.HCNUMVER < 90800 }}
And here's a live demonstration featuring coomparison against a number like 9.10.7.
I have also now added a golden test.
| , fixtureGoldenTest "copy-fields-some" | ||
| , fixtureGoldenTest "copy-fields-none" | ||
| ] | ||
| , testGroup "github-specific" |
There was a problem hiding this comment.
unnecessary complication.
Please, do the least required stuff. I'd tell if it's not enough, now it's too much.
There was a problem hiding this comment.
Do you mean, remove the extra testGroup "github-specific" layer, but keep fixtureWeederGoldenTest? I've removed the extra group layer now.
There was a problem hiding this comment.
Or if you mean that fixtureWeederGoldenTest is too complicated, could you help give an idea of what a minimal test would look like?
Closes #132
In this PR I also parameterized the
freeToArithfunction to allow for formatting an expression according to GitHub actions expression syntax.Test procedure
Example output: swarm-game/swarm#2059