Skip to content

Conversation

@tullom
Copy link
Contributor

@tullom tullom commented Dec 10, 2025

This pull request introduces several improvements and fixes to the project, focusing on error handling in the register interface, lint configuration, and CI workflow enhancements. The most significant changes are grouped below by theme.

Error Handling and Robustness:

  • Added a new RegisterSizeError variant to the BQ25773Error enum and updated the async register write implementation in src/lib.rs to explicitly check buffer sizes and return this error if the data does not fit, instead of using debug_assert!. This improves runtime safety and error reporting. [1] [2] [3]
  • Updated tests to allow clippy::unwrap_used, ensuring that test code does not fail linting due to unwrap usage.

Lint and Metadata Improvements:

  • Refined Cargo.toml keywords to better reflect the crate’s scope, and updated clippy lint settings to use stricter enforcement (deny instead of forbid) and to add more lint categories for improved code quality. [1] [2]

Continuous Integration Enhancements:

  • Enabled the semver job in the GitHub Actions workflow (.github/workflows/check.yml) to check for semantic versioning compatibility on each run, improving release safety.

@tullom tullom self-assigned this Dec 10, 2025
@tullom tullom added the bug Something isn't working label Dec 10, 2025
@tullom tullom requested a review from a team as a code owner December 10, 2025 17:31
@tullom tullom added the enhancement New feature or request label Dec 10, 2025
kurtjd
kurtjd previously approved these changes Dec 10, 2025
Copy link

@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.

Looks good, though this counts as a breaking change I believe since the BQ25773Error is not marked non_exhaustive.

@tullom
Copy link
Contributor Author

tullom commented Dec 10, 2025

Looks good, though this counts as a breaking change I believe since the BQ25773Error is not marked non_exhaustive.

I realized i never published this crate, so i added a commit here that enables the semver check. It will fail for now, but once i get 2 approvals, i will cargo publish and re-run CI and i should be able to merge then

RobertZ2011
RobertZ2011 previously approved these changes Dec 10, 2025
asasine
asasine previously approved these changes Dec 12, 2025
Copy link

@asasine asasine left a comment

Choose a reason for hiding this comment

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

non-blocking

@jerrysxie jerrysxie self-requested a review December 15, 2025 17:48
kurtjd
kurtjd previously approved these changes Dec 15, 2025
@jerrysxie jerrysxie enabled auto-merge (squash) December 16, 2025 18:36
@jerrysxie jerrysxie disabled auto-merge December 16, 2025 18:36
@jerrysxie jerrysxie merged commit 8047903 into OpenDevicePartnership:main Dec 16, 2025
8 checks passed
@github-project-automation github-project-automation bot moved this from In review to Done in Embedded Controller Dec 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants