Add basic CI #18
Add basic CI #18
Conversation
JakeHartnell
force-pushed
the
JakeHartnell/add-basic-ci
branch
3 times, most recently
from
de13838 to
1657c7f
Compare
Closes #15
JakeHartnell
force-pushed
the
JakeHartnell/add-basic-ci
branch
from
1657c7f to
d5e2c75
Compare
ethanfrey
left a comment
ethanfrey
left a comment
There was a problem hiding this comment.
Can you please not use nightly in CI.
Let's use a fixed stable version, like we will use for really developing and compiling them.
.github/workflows/basic.yml
Outdated
| - name: Checkout sources | ||
| uses: actions/checkout@v2 | ||
|
|
||
| - name: Install latest nightly toolchain |
There was a problem hiding this comment.
Let's not use nightly.
Code should work with some well-defined version of Rust and only use nightly briefly if we really need some feature that is about to be stabilized.
I think MSRV for cosmwasm is 1.65. Latest release is 1.70.
We can use anything between that (inclusive) for this CI. But make it explicit and then we make a PR to update.
Sometimes newer version flag lint errors earlier ones didn't and randomly breaking CI (when a new rust docker image is pushed) is not so much fun.
.github/workflows/basic.yml
Outdated
| - name: Run tests | ||
| uses: actions-rs/cargo@v1 | ||
| with: | ||
| toolchain: nightly |
There was a problem hiding this comment.
I would remove toolchain: nightly everywhere and put whichever version we need. I think just stable
| target: wasm32-unknown-unknown | ||
| override: true | ||
|
|
||
| - name: Run tests |
There was a problem hiding this comment.
Nit, I often include a build phase before test.
If there are build errors and I run test, sometimes they seem to be reported twice.
I mean, it will fail regardless, but seems to give better output running them one after another
| env: | ||
| RUST_BACKTRACE: 1 | ||
|
|
||
| - name: Compile WASM contract |
There was a problem hiding this comment.
Nit: this compiles whole workspace and not really good at checking contracts.
You could try something like https://github.com/CosmWasm/cw-plus/blob/main/.circleci/config.yml#L500-L519
If something gets imported with library feature flag, then it will show up as super small contract and missing entry points (where cosmwasm-check will fail). Nicer to have this fail on the PR that changes the imports, then right before a release.
(You can make those additions as a future PR)
.github/workflows/basic.yml
Outdated
| uses: actions-rs/toolchain@v1 | ||
| with: | ||
| profile: minimal | ||
| toolchain: nightly |
There was a problem hiding this comment.
How about stable (to match the name).
Ideally use a fixed tag, so CI doesn't break when new stable comes out
ethanfrey
left a comment
ethanfrey
left a comment
There was a problem hiding this comment.
Great changes.
Question for future: can we put 1.65.0 in some variable for reuse?
Closes #15