Skip to content

Fix log messages which are not format strings#15962

Merged
raunaqmorarka merged 1 commit intotrinodb:masterfrom
ksobolew:kudi/format-method-loggers
Feb 10, 2023
Merged

Fix log messages which are not format strings#15962
raunaqmorarka merged 1 commit intotrinodb:masterfrom
ksobolew:kudi/format-method-loggers

Conversation

@ksobolew
Copy link
Copy Markdown
Contributor

@ksobolew ksobolew commented Feb 3, 2023

Description

See also: 2ec0529

This is currently not enforced in Error Prone because it requires annotating more methods in airlift:log with @FormatMethod, but that's hard to do, because it will change their semantics.

Additional context and related issues

Extracted from #14933.

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

Copy link
Copy Markdown
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

%

See also: 2ec0529

This is currently not enforced in Error Prone because it requires
annotating more methods in `airlift:log` with `@FormatMethod`, but
that's hard to do, because it will change their semantics.
@ksobolew ksobolew force-pushed the kudi/format-method-loggers branch from c47502a to eb872da Compare February 3, 2023 13:48
Copy link
Copy Markdown
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

thanks!

@raunaqmorarka raunaqmorarka merged commit 93becc1 into trinodb:master Feb 10, 2023
@github-actions github-actions Bot added this to the 407 milestone Feb 10, 2023
@ksobolew ksobolew deleted the kudi/format-method-loggers branch February 10, 2023 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants