Combine PRs for ease of merging #423
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tcharding
changed the title
tcharding
force-pushed
the
ci-fixes-combine
branch
from
bf6bd27 to
76f66d7
Compare
Remove single character of trailing whitespace.
The test that checks for a panic uses `cargo test --exact`, it makes sense to put it at the top of the script right after we run `cargo test` so we can run the tests without triggering a re-build.
In line with the `Tests` job and for the facot that this job does stuff with the nightly toolchain other than bench. Re-name nightly CI job from `bench_nightly`to `Nightly`.
We use a matrix with a single element, this is unnecessary.
The address sanitizer job is silently failing at the moment because we do not install clang. Install clang so the address sanitizer job can run. Do not fix the silent failure, that will be done later on.
We have a separate CI job for things that require a nightly toolchain. Building the docs requires a nightly toolchain (because of `--cfg docsrc` flag). It makes more sense to run the docs build in the `Nightly` job instead of hidden in the `Tests` job. Do the docs build in the `Nightly` job instead of in the `Tests` job.
WASM is supported by Rust 1.30. We can therefore run the WASM tests on any all the toolchains except MSRV (1.29.0). This has benefit of catching nightly/beta issues before they get to stable. Done as a separate CI job since it is conceptually different to the `Tests` job. Run WASM for nightly, beta, and stable toolchains.
The const definitions in `stdio.h` are causing the `wasm-pack` build to fail due to 'duplicate symbol' errors. This can be solved by just removing the definitions.
As per UNIX convention a Bash script should exit with status code 0 if it completes successfully.
Currently the `test.sh` script is silently failing because we do not exit if a command fails. We can achieve this by using the Bash builtin `set -e`. For some reason I cannot explain a chain of commands that fails does not fail the script. Instead of working out _why_ just remove the chain and run each command on its own. This is functionally the same and, I hazard a guess, is what the original author hoped to achieve with the chaining.
tcharding
force-pushed
the
ci-fixes-combine
branch
from
76f66d7 to
ca052ee
Compare
tcharding
changed the title
A user reported that WASM only used to work with `clang-10`, I cannot verify this but lets use v10 anyways.
tcharding
force-pushed
the
ci-fixes-combine
branch
from
ca052ee to
d4d35d1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is #420, #421, and #422 applied in order.
Confused yet?
I've combined them because WASM is currently broken on
master, the individual PRs are easier to review but this PR is better for merging because it shows that they all work when applied in order.Fixes: #419