Skip to content

Fix clippy suppression in defmt logging#750

Merged
kurtjd merged 1 commit intoOpenDevicePartnership:mainfrom
kurtjd:log-clippy-fix
Mar 16, 2026
Merged

Fix clippy suppression in defmt logging#750
kurtjd merged 1 commit intoOpenDevicePartnership:mainfrom
kurtjd:log-clippy-fix

Conversation

@kurtjd
Copy link
Contributor

@kurtjd kurtjd commented Mar 16, 2026

In #278 I may have been too aggressive in how I was trying to suppress clippy lints stemming from the defmt logging macro. Instead, I just do an empty let binding on the string literal instead of all the arguments, which shouldn't result in any possible side-effects since a string literal should always be able to be optimized away if bound to a _.

Note: The context is that the clippy suppressions introduced are potentially causing adverse side-effects in production code hence the need for this change.

@kurtjd kurtjd requested review from JamesHuard and jerrysxie March 16, 2026 16:24
@kurtjd kurtjd self-assigned this Mar 16, 2026
@kurtjd kurtjd requested a review from a team as a code owner March 16, 2026 16:24
@kurtjd kurtjd added the bug Something isn't working label Mar 16, 2026
@github-project-automation github-project-automation bot moved this to In progress in ODP Backlog Mar 16, 2026
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

Adjusts the logging macro implementations in embedded-service to avoid clippy-suppression patterns that can unintentionally evaluate logging arguments (and thus cause side effects) when logging is filtered/disabled.

Changes:

  • Removed the let _ = (...) “argument binding” from the defmt + log (host) macro variants.
  • Replaced the let _ = ($s, $( &$x ),*) binding with a no-argument-evaluation let _ = $s; in the defmt-only macro variants.

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

You can also share your feedback on Copilot code review. Take the survey.

@jerrysxie
Copy link
Contributor

@damonhunka Can you give this a try to see if it resolves your problem?

@kurtjd kurtjd enabled auto-merge (squash) March 16, 2026 17:16
@kurtjd kurtjd merged commit 2bd6fdf into OpenDevicePartnership:main Mar 16, 2026
15 checks passed
@github-project-automation github-project-automation bot moved this from In progress to Done in ODP Backlog Mar 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants