Skip to content

Conversation

@tnk4on
Copy link
Contributor

@tnk4on tnk4on commented Jan 9, 2026

Fix clippy::disallowed_methods warnings that appear in clean builds but may be hidden by CI cache.

Changes

  • crates/lib/src/status.rs: Replace label.len() with label.as_bytes().len()
  • crates/tests-integration/src/container.rs: Replace digest.len() with digest.as_bytes().len()

Background

The project's clippy.toml disallows str::len() to make the intent of getting byte length explicit.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request replaces str::len() with str::as_bytes().len() to comply with a project-specific clippy lint. While the change in crates/tests-integration/src/container.rs is correct for a hex string, the change in crates/lib/src/status.rs introduces a latent bug. It calculates the maximum label length in bytes, but the corresponding write_row_name function calculates padding based on character count. This will cause misaligned text if labels contain multi-byte Unicode characters. My review includes a suggestion to use chars().count(), which correctly measures character count for alignment purposes and should also satisfy the clippy lint.

@tnk4on tnk4on force-pushed the fix-clippy-warnings branch from 7f237bc to e0d64cd Compare January 9, 2026 16:03
cgwalters
cgwalters previously approved these changes Jan 9, 2026
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Great thank you!

@cgwalters cgwalters enabled auto-merge (rebase) January 9, 2026 16:05
- status.rs: Use UnicodeWidthStr::width() for correct display alignment
- container.rs: Use as_bytes().len() for hex string length verification
- Add unicode-width dependency (already a transitive dep via comfy-table)

Assisted-by: Cursor (Auto)
Signed-off-by: Shion Tanaka <shtanaka@redhat.com>
auto-merge was automatically disabled January 9, 2026 16:24

Head branch was pushed to by a user without write access

@tnk4on tnk4on force-pushed the fix-clippy-warnings branch from e0d64cd to 9cfa2ca Compare January 9, 2026 16:24
@cgwalters
Copy link
Collaborator

And I guess a side thing to investigate here is why our CI is not properly gating on this, probably something missing in just validate

@cgwalters cgwalters enabled auto-merge (rebase) January 9, 2026 19:17
@cgwalters cgwalters merged commit ab78a79 into bootc-dev:main Jan 9, 2026
48 checks passed
@tnk4on tnk4on deleted the fix-clippy-warnings branch January 9, 2026 19:43
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