Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimize:formating log messages #1726

Merged
merged 1 commit into from
Oct 7, 2019
Merged

optimize:formating log messages #1726

merged 1 commit into from
Oct 7, 2019

Conversation

zaqweb
Copy link
Contributor

@zaqweb zaqweb commented Oct 4, 2019

Ⅰ. Describe what this PR did

This PR is just replace the parameters from string concatenation to message formatting

LOGGER.error("rpcContext is null,channel:" + channel + ",active:" + channel.isActive());

change to:

LOGGER.error("rpcContext is null,channel:{},active:{}", channel, channel.isActive());

Ⅱ. Does this pull request fix one issue?

no

Ⅲ. Why don't you add test cases (unit test/integration test)?

It could be verified by code review

Ⅳ. Describe how to verify it

code review and the actual logs

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Oct 4, 2019

Codecov Report

Merging #1726 into develop will decrease coverage by 0.13%.
The diff coverage is 10%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1726      +/-   ##
=============================================
- Coverage      47.01%   46.87%   -0.14%     
  Complexity      1786     1786              
=============================================
  Files            365      365              
  Lines          13301    13300       -1     
  Branches        1644     1617      -27     
=============================================
- Hits            6253     6234      -19     
- Misses          6370     6371       +1     
- Partials         678      695      +17
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/io/seata/core/rpc/ChannelManager.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...java/io/seata/core/rpc/netty/RpcClientHandler.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...ata/core/rpc/DefaultServerMessageListenerImpl.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...eata/core/rpc/netty/NettyClientChannelManager.java 71.17% <40%> (+0.63%) 22 <0> (ø) ⬇️
...obuf/convertor/BranchRegisterRequestConvertor.java 88% <0%> (-8%) 3% <0%> (ø)
...buf/convertor/GlobalLockQueryRequestConvertor.java 92.59% <0%> (-7.41%) 3% <0%> (ø)
...obuf/convertor/GlobalRollbackRequestConvertor.java 94.73% <0%> (-5.27%) 3% <0%> (ø)
...otobuf/convertor/GlobalStatusRequestConvertor.java 94.73% <0%> (-5.27%) 3% <0%> (ø)
...otobuf/convertor/GlobalCommitRequestConvertor.java 94.73% <0%> (-5.27%) 3% <0%> (ø)
...protobuf/convertor/RegisterTMRequestConvertor.java 95.45% <0%> (-4.55%) 3% <0%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29e1928...26c9ff5. Read the comment docs.

@lovepoem lovepoem added the first-time contributor first-time contributor label Oct 4, 2019
Copy link
Member

@xingfudeshi xingfudeshi left a comment

Choose a reason for hiding this comment

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

LGTM.

@xingfudeshi xingfudeshi changed the title user message formatting as logger param instead of string concatenation optimize:fommating log messages Oct 6, 2019
@xingfudeshi xingfudeshi changed the title optimize:fommating log messages optimize:formating log messages Oct 6, 2019
Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

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

LGTM

@slievrly slievrly merged commit 36042d6 into apache:develop Oct 7, 2019
@wangliang181230 wangliang181230 added this to the 0.9.0 milestone Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-time contributor first-time contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants