◐ Shell
reader mode source ↗
Skip to content

Resolve cargo-shear warnings#8031

Open
ShaharNaveh wants to merge 9 commits into
RustPython:mainfrom
ShaharNaveh:cargo-shear-comments
Open

Resolve cargo-shear warnings#8031
ShaharNaveh wants to merge 9 commits into
RustPython:mainfrom
ShaharNaveh:cargo-shear-comments

Conversation

@ShaharNaveh

@ShaharNaveh ShaharNaveh commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Summary by CodeRabbit

  • Removals

    • Disabled the optional Tkinter UI’s platform dependency path and excluded related runtime pieces from builds.
    • Disabled Rustls TLS backend support that depends on platform verification and X509 certificates.
  • Chores

    • Adjusted dependency wiring so one TLS backend is used only in development contexts.
    • Disabled several TLS/X509/Tk-related workspace dependencies.
    • Normalized feature-flag declarations and manifest formatting across crates.
    • Explicitly kept an unused runtime module out of all builds.

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 01478ffb-54a2-448b-816e-7b685a4213f9

📥 Commits

Reviewing files that changed from the base of the PR and between 9cea770 and c44f6c4.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (1)
  • Cargo.toml

📝 Walkthrough

Walkthrough

Workspace TLS and Tk/Tcl workspace dependencies are disabled and rustls-graviola is moved to dev-dependencies; crates/stdlib and crates/codegen update feature lists and dependency entries accordingly, and crates/stdlib/src/lib.rs adds an explicit #[cfg(false)] module guard.

Changes

Feature and Dependency Cleanup

Layer / File(s) Summary
Workspace dependency disabling and dev-dependency reorganization
Cargo.toml
Removed optional rustls-graviola from root [dependencies], added it under [dev-dependencies], and commented out workspace rustls-platform-verifier, tcl-sys, and x509-cert.
stdlib feature and dependency adjustments
crates/stdlib/Cargo.toml
Reformatted tkinter and __ssl-rustls features to multiline arrays, disabled dep:tcl-sys in the tkinter feature, commented out tcl-sys, rustls-platform-verifier, and x509-cert dependencies, and normalized chrono = { workspace = true }.
codegen std feature simplification
crates/codegen/Cargo.toml
Reformatted the std feature to multiline, left "thiserror/std" enabled and commented out itertools/use_std, and commented out the itertools dependency in [dependencies].
Module compilation guard formalization
crates/stdlib/src/lib.rs
Replaced a commented-out mod re; with #[cfg(false)] mod re; to ensure the module is excluded from all builds.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • RustPython/RustPython#7747: Touches the same Cargo dependencies for Tk/Tcl and Rustls certificate verification (tcl-sys, tk-sys, x509-cert, rustls-platform-verifier).
  • RustPython/RustPython#7733: Related changes to TLS backend wiring that affect SSL-enabled vs SSL-disabled CI/testing paths.
  • RustPython/RustPython#7786: Refactors crates/stdlib/src/lib.rs Rustls/SSL module gating; both PRs target the same feature stack and module organization.

Suggested reviewers

  • fanninpm

Poem

🐰 I pruned the crates and set them right,
TLS and Tcl tucked out of sight,
rustls-graviola now naps in dev,
re sleeps under #[cfg(false)]'s rev,
Hooray for tidy trees tonight. 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Resolve cargo-shear warnings' directly aligns with the PR's main objective of addressing cargo-shear warnings through dependency and feature configuration changes across multiple Cargo.toml files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ShaharNaveh ShaharNaveh marked this pull request as ready for review June 3, 2026 14:07

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Cargo.toml`:
- Line 64: The example `custom_tls_providers` is gated on a non-existent feature
name `rustls-graviola` because `rustls-graviola` is only in dev-dependencies and
not declared as an optional dependency/feature; fix by either moving
`rustls-graviola` from `[dev-dependencies]` into `[dependencies]` with `optional
= true` so Cargo creates a `rustls-graviola` feature, or remove
`"rustls-graviola"` from the example's `required-features` and instead gate
`custom_tls_providers` on one of the existing declared features (e.g.,
`rustls/ring`, `ssl-rustls`) in Cargo.toml; update the `[[example]]
custom_tls_providers` stanza and the dependency declaration for
`rustls-graviola` accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 10cfb8ab-63e2-4e33-a992-ebfb19c41e81

📥 Commits

Reviewing files that changed from the base of the PR and between c5f555e and f82c038.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • Cargo.toml
  • crates/codegen/Cargo.toml
  • crates/stdlib/Cargo.toml
  • crates/stdlib/src/lib.rs

@ShaharNaveh ShaharNaveh requested a review from youknowone June 16, 2026 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants