Reflect scripts as bb tasks #63
Conversation
|
@ikappaki I prefer to put tasks that are longer than say 3 lines, as functions in a The |
|
A small example of that here: https://github.com/squint-cljs/squint/blob/main/bb.edn |
Sure, I've just move the biggest offender to
I plan to upgrade to the latest graalvm version with your blessing after the CI move to bb, so if you agree to it I'll make sure I'll look out for dead options. |
|
Sounds good |
|
Can you activate this script in CI, so it can be tested before merging? |
Hi @borkdude, I've replaced the test/compile scripts with bb tasks in the CI scripts, and removed them from the codebase (since they are tested to work). May I leave the The next step after this PR is to consolidate the builds under one CI system. Given there could be different argument quoting behavior between the supported java version, I'm thinking of building a matrix of [macos, ubuntu, windows]x[jdk 1.8, 11, 15, 17] to test all possible outcomes. I can do this in a separate PR. Do you have a particular preference on the CI system to use? (I've also sneaked in an update to Thanks, |
appveyor.yml
Outdated
| @@ -16,9 +16,12 @@ cache: | |||
|
|
|||
| build_script: | |||
| - cmd: >- | |||
| powershell -Command "(New-Object Net.WebClient).DownloadFile('https://raw.githubusercontent.com/technomancy/leiningen/stable/bin/lein.bat', 'lein.bat')" | |||
| choco install lein | |||
There was a problem hiding this comment.
I'm not a fan of making changes to CI builds if they are not necessary. The powershell command worked before. Switching to choco may slow down stuff or additional stuff may be installed which causes new unforeseen issues. The reason I'm saying this is that I've used brew on macOS CI and this was very very slow.
There was a problem hiding this comment.
Indeed, I forgot to mention that I did that because the compile script only looks in PATH to locate lein, and not in the CWD as per the above installation does. I had a few futile attempts trying to add it in the appveyor PATH and in the end settled at using choco (courtesy of grep.app).
Understood. I've reverted the change and updated the compile task to look at CWD for lein. There is an inherent difficulty trying to locate windows executable programs/scripts outside of PATH without mentioning the extension. I might open a babashka.fs to discuss the problem.
Thanks
|
I'm only using script/bump_version.clj and script\changelog.clj locally, so it's ok to not touch those. |
I'm fine with adding Github actions for the test matrix. If the native build is sufficiently fast, compared to appveyor, we could switch, but let's leave it in for now. |
`bb compile` should also search for lein in the CWD for compatibility with the appveyor job.
|
Thanks! |
Hi,
could you please review patch to reflect all
script/s as bb tasks. This is part of #61.I've noticed there were a couple of graalvm options missing from the windows
compilescript (-H:IncludeResources=DEPS_CLJ_VERSIONand--no-server), I used the bashcompilescript as a reference. I've also removed the command to install graalvm's native image program, because I feel this is more of a workstation setup rather than the responsibility of the compile task ...If this is approved, I would like to update the CI jobs next to reference the bb tasks and remove the old scripts from the codebase. I could either do this as part of this PR or raise a new one.
Thanks,