-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fs: emit compilation error without tokio_unstable
for io-uring
#7634
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
tokio_unstable
for io-uring
a579578
to
d0c7c35
Compare
/// panic!("This panic will shutdown the runtime."); | ||
/// }).await; | ||
/// } | ||
/// # #[cfg(not(tokio_unstable))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// }).await; | ||
/// }) | ||
/// } | ||
/// # #[cfg(not(tokio_unstable))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: ADD-SP <qiqi.zhang@konghq.com>
d0c7c35
to
33c603b
Compare
Sorry for the delay, I'll check this tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change makes CI code cleaner, thanks! I left some comments but mostly lgtm.
.github/workflows/ci.yml
Outdated
cargo nextest run --all-features | ||
cargo test --doc --all-features | ||
# taskdump is an unstable feature, but it can only be enabled | ||
# by --cfg tokio_taskdump, not by a feature flag, so we can | ||
# use $TOKIO_STABLE_FEATURES here. | ||
cargo nextest run --features $TOKIO_STABLE_FEATURES | ||
cargo test --doc --features $TOKIO_STABLE_FEATURES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this part really use the taskdump feature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- name: "clippy --all ${{ matrix.rustflags }}" | ||
run: cargo clippy --all --tests --all-features --no-deps | ||
- name: "clippy --all --features ${{ env.TOKIO_STABLE_FEATURES }}" | ||
run: cargo clippy --all --tests --no-deps --features $TOKIO_STABLE_FEATURES | ||
- name: "clippy --all --all-features --unstable" | ||
run: cargo clippy --all --tests --no-deps --all-features | ||
env: | ||
RUSTFLAGS: ${{ matrix.rustflags }} | ||
RUSTFLAGS: --cfg tokio_unstable --cfg tokio_taskdump -Dwarnings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, this change removed the case where we don't use tokio_unstable
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a cargo cliipy
in the near above, did you mean this?
- name: "clippy --all --features ${{ env.TOKIO_STABLE_FEATURES }}"
run: cargo clippy --all --tests --no-deps --features $TOKIO_STABLE_FEATURES
Signed-off-by: ADD-SP <qiqi.zhang@konghq.com>
The fixup of #7621 for better error message. Otherwise, the error message is a bit encrypted.
This is a bit complicated. It seems like it's not worth the complexity, but we still have to do it because we also want to convert
--cfg tokio_taskdump
into a Cargo feature, and taskdump already has similar user-friendly error messages.tokio/tokio/src/lib.rs
Lines 477 to 478 in 3b5a15d