Skip to content

Fix Clippy and CI on current#3549

Draft
meditationmind wants to merge 1 commit intoserenity-rs:currentfrom
meditationmind:Fix-Clippy-and-MSRV-CI-on-current
Draft

Fix Clippy and CI on current#3549
meditationmind wants to merge 1 commit intoserenity-rs:currentfrom
meditationmind:Fix-Clippy-and-MSRV-CI-on-current

Conversation

@meditationmind
Copy link
Copy Markdown
Contributor

Fixed Clippy and MSRV CI errors on current.

@github-actions github-actions Bot added the model Related to the `model` module. label May 4, 2026
@meditationmind meditationmind force-pushed the Fix-Clippy-and-MSRV-CI-on-current branch from a42f9e8 to ba9d865 Compare May 4, 2026 09:40
@github-actions github-actions Bot added the framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate label May 4, 2026
@meditationmind meditationmind marked this pull request as draft May 4, 2026 10:36
@meditationmind meditationmind force-pushed the Fix-Clippy-and-MSRV-CI-on-current branch from ba9d865 to b6f085c Compare May 4, 2026 10:38
@meditationmind meditationmind marked this pull request as ready for review May 4, 2026 13:06
@meditationmind
Copy link
Copy Markdown
Contributor Author

Hope this was the right way to approach this.

The rustc-hash change is to preserve MSRV (2.1.2 bumps MSRV to 1.77). reqwest started pulling in a copy of hyper-rustls that breaks MSRV (0.27.8 bumps MSRV to 1.85), so I added the compatible version to the rustls_backend feature. That broke the minimal versions CI, so I also added the minimum version of hyper-util to the feature to fix that.

@mkrasnitski
Copy link
Copy Markdown
Collaborator

Our approach to supporting an MSRV as old as 1.74 is more hands off - we maintain a list of locked versions in .github/workflows/ci.yml which allows the MSRV job to succeed, and don't claim to support building with the MSRV out of the box without those version pins. I tested on my own fork, and seems like adding the following pins is sufficient to build:

cargo update hyper-rustls@0.27 --precise 0.27.7
cargo update rustc-hash --precise 2.1.1

The changes to Cargo.toml itself aren't necessary.

@meditationmind meditationmind force-pushed the Fix-Clippy-and-MSRV-CI-on-current branch from b6f085c to 2198bd7 Compare May 6, 2026 03:34
@github-actions github-actions Bot added the ci Related to the continuous integration. label May 6, 2026
@meditationmind meditationmind force-pushed the Fix-Clippy-and-MSRV-CI-on-current branch 2 times, most recently from a38d744 to b8243ac Compare May 6, 2026 06:05
@meditationmind meditationmind marked this pull request as draft May 6, 2026 06:38
@meditationmind meditationmind force-pushed the Fix-Clippy-and-MSRV-CI-on-current branch from b8243ac to 2173e9f Compare May 6, 2026 11:05
@github-actions github-actions Bot added client Related to the `client` module. utils Related to the `utils` module. labels May 6, 2026
@meditationmind meditationmind force-pushed the Fix-Clippy-and-MSRV-CI-on-current branch from 2173e9f to e7b2e51 Compare May 6, 2026 11:10
@github-actions github-actions Bot removed client Related to the `client` module. utils Related to the `utils` module. labels May 6, 2026
@meditationmind
Copy link
Copy Markdown
Contributor Author

Ah, yes, that's a much better solution! I knew Poise used the MSRV-specific Cargo.lock, but I hadn't noticed the pins in the workflow file for Serenity. Thanks!

Unfortunately, something seems to have changed in nightly over the past few days, and Clippy is now complaining about the visibility of error::Result and error::Error.

error[E0365]: `Result` is only public within the crate, and cannot be re-exported outside
   --> src/lib.rs:185:9
    |
185 |         model::prelude::*,
    |         ^^^^^^^^^^^^^^^^^ re-export of crate public `Result`
    |
    = note: consider declaring type or module `Result` with `pub`

Even though they're public, they have a pub use in internal::prelude, which is pub(crate), and that seems to be causing issues since it's included in model::prelude. I'm assuming the error module has been intentionally kept private to allow for controlling where and how Result and Error are exported. I tried adjusting visibility in several places, but all of the re-exporting via various preludes is a bit of a maze and I keep running into new errors.

I've set this back to draft for now. Any advice would be appreciated.

@mkrasnitski
Copy link
Copy Markdown
Collaborator

This is in fact a rustc bug regressed in the newest nightly, and I used cargo bisect-rustc to bisect to rust-lang/rust#156014. Trying to write a minimal repro atm, but not having any luck.

@meditationmind
Copy link
Copy Markdown
Contributor Author

Wow, thanks for the bisect tip. I spent a while manually combing through PRs trying to find the source (without much success).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Related to the continuous integration. framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate model Related to the `model` module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants