Conversation
waywardmonkeys
force-pushed
the
test-no_std-in-ci
branch
from
146c1ac to
211d7c6
Compare
|
so, |
We don't want these on examples and such.
waywardmonkeys
force-pushed
the
test-no_std-in-ci
branch
from
211d7c6 to
e899be5
Compare
DJMcNab
left a comment
DJMcNab
left a comment
There was a problem hiding this comment.
I'm definitely happy to merge with a new variable added. Happy to alternatively see what Kaur thinks
| save-if: ${{ github.event_name != 'merge_group' }} | ||
|
|
||
| # TODO: Add --target x86_64-unknown-none to the no_std check once we solve the compilation issues with it. | ||
| # https://github.com/linebender/parley/issues/86 |
There was a problem hiding this comment.
What's the status of #86 once this PR is merged? I don't think this is currently marked as fixing that issue.
There was a problem hiding this comment.
Needs to be reviewed!
| # https://github.com/linebender/parley/issues/86 | ||
| - name: cargo clippy (no_std) | ||
| run: cargo hack clippy --workspace --locked --optional-deps --each-feature --ignore-unknown-features --features libm --exclude-features ${{ env.FEATURES_DEPENDING_ON_STD }} -- -D warnings | ||
| run: cargo hack clippy ${{ env.RUST_MIN_VER_PKGS }} --locked --optional-deps --each-feature --ignore-unknown-features --features libm --exclude-features ${{ env.FEATURES_DEPENDING_ON_STD }} --target x86_64-unknown-none -- -D warnings |
There was a problem hiding this comment.
Hmm. I think the idiomatic approach here would be to add a new environment variable? cc @xStrom
There was a problem hiding this comment.
An even better solution is a brand new cargo-hack feature --must-have-and-exclude-feature std. Unfortunately it's been stuck in review queue for a month at cargo-hack#262.
For now I think it's fine to abuse ${{ env.RUST_MIN_VER_PKGS }} and just get this landed. When the proper solution becomes possible we'll adjust it.
xStrom
left a comment
xStrom
left a comment
There was a problem hiding this comment.
Yes this contains a hack in the form of abusing ${{ env.RUST_MIN_VER_PKGS }}, but the new state is still much better than the old one so let's land this.
|
Until I got distracted by work, I was going to suggest that |
|
One issue is that we could introduce published packages that don't support So my recommendation is to still just merge this as-is to get things moving, and the eventual standard change would be the more robust |
3 participants
This builds on #238 and contains the same commit.
This has a change to only run
no_stdchecks on the libraries since examples and such aren't likely to beno_std, nor do they need to be.