Skip to content

Conversation

@jerrysxie
Copy link
Contributor

@jerrysxie jerrysxie commented Dec 23, 2025

This pull request introduces stricter Clippy lint enforcement at the project level and updates how Clippy lints are handled in the CI script. The main goal is to ensure code quality by denying a wide range of lints by default, while simplifying the Clippy invocation in CI. Additionally, a specific lint is suppressed for the espi module.

Lint configuration improvements:

  • Added a [lints.clippy] section to Cargo.toml to deny several categories of Clippy lints globally, including correctness, expect_used, indexing_slicing, panic, perf, suspicious, style, todo, unimplemented, unreachable, and unwrap_used.

  • Added #[allow(clippy::indexing_slicing)] to the espi module declaration in src/lib.rs to suppress this lint specifically for that module.

CI configuration changes:

  • Simplified the Clippy invocation in ci.sh by removing explicit lint deny flags from the command line, relying instead on the lints configured in Cargo.toml.

@jerrysxie jerrysxie self-assigned this Dec 23, 2025
Copilot AI review requested due to automatic review settings December 23, 2025 20:48
@jerrysxie jerrysxie added the github_actions Pull requests that update GitHub Actions code label Dec 23, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds panic-related clippy lints to the CI pipeline by configuring them in Cargo.toml rather than as command-line flags. The eSPI module is excluded from the indexing_slicing lint as noted in the PR description.

  • Centralizes clippy lint configuration in Cargo.toml with 11 strict lints including panic detection and code quality rules
  • Simplifies CI clippy commands to rely on the workspace lint configuration instead of explicit command-line flags
  • Excludes the eSPI module from indexing_slicing lint enforcement

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
Cargo.toml Adds [lints.clippy] section with 11 deny-level lints for panic detection, code quality, and best practices
src/lib.rs Adds #[allow(clippy::indexing_slicing)] attribute to espi module to exclude it from the new lint
ci.sh Simplifies clippy commands from explicit lint flags to just -Dwarnings, relying on Cargo.toml configuration
.github/workflows/check.yml Simplifies clippy command for examples from explicit lint flags to just -Dwarnings

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

kurtjd
kurtjd previously approved these changes Dec 23, 2025
Copy link
Contributor

@kurtjd kurtjd left a comment

Choose a reason for hiding this comment

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

It's odd that keyboard-service and thermal-service don't generate panic lint warnings here in CI (they do for me locally). Regardless could make sense to exclude them as well for the time-being since they aren't used externally.

* Exclude eSPI module as no external client yet
Copilot AI review requested due to automatic review settings December 24, 2025 00:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jerrysxie jerrysxie marked this pull request as ready for review December 24, 2025 01:11
@jerrysxie jerrysxie requested a review from a team as a code owner December 24, 2025 01:11
@jerrysxie jerrysxie enabled auto-merge (squash) December 24, 2025 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github_actions Pull requests that update GitHub Actions code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants