Skip to content

Conversation

tklauser
Copy link
Member

Use the correct format verb when logging error messages (%s instead of %w) and fix an error message related to requireModeIsNot requirements in feature detection.

See commits for details.

@tklauser tklauser requested review from a team as code owners June 18, 2025 11:34
@tklauser tklauser added kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. cilium-cli This PR contains changes related with cilium-cli labels Jun 18, 2025
@tklauser tklauser requested a review from bimmlerd June 18, 2025 11:34
@tklauser tklauser added the cilium-cli-exclusive This PR only impacts cilium-cli binary label Jun 18, 2025
@tklauser tklauser requested review from Artyop and derailed June 18, 2025 11:34
@tklauser tklauser enabled auto-merge June 18, 2025 11:35
@tklauser
Copy link
Member Author

/test

Copy link
Member

@bimmlerd bimmlerd left a comment

Choose a reason for hiding this comment

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

thanks!

tklauser added 2 commits June 18, 2025 13:55
Don't use the %w format verb in log messages, otherwise the error
will be wrapped and will show up as follows in the respective log:

   %!w(*errors.errorString=&{some error message})

Use the %s format verb instead to just add the error message to the log
output.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
Remove a stray argument that isn't supposed to be part of the error
message and make the message a bit terser by not repeating the feature
name.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser tklauser force-pushed the pr/tklauser/cilium-cli-error-msgs branch from 0c78f3d to 316e23e Compare June 18, 2025 11:55
@tklauser tklauser requested a review from a team as a code owner June 18, 2025 11:55
@tklauser tklauser requested a review from HadrienPatte June 18, 2025 11:56
@tklauser
Copy link
Member Author

/test

Copy link
Member

@HadrienPatte HadrienPatte left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@bimmlerd bimmlerd left a comment

Choose a reason for hiding this comment

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

reapproving for policy

Copy link
Contributor

@derailed derailed left a comment

Choose a reason for hiding this comment

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

@tklauser Good catch. Thank you!

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 19, 2025
@tklauser tklauser added this pull request to the merge queue Jun 19, 2025
Merged via the queue into main with commit bf22730 Jun 19, 2025
294 of 306 checks passed
@tklauser tklauser deleted the pr/tklauser/cilium-cli-error-msgs branch June 19, 2025 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary kind/cleanup This includes no functional changes. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants