twitter / util

Wonderful reusable code from Twitter
https://twitter.github.io/util
Apache License 2.0
2.69k stars 581 forks source link

add missing type ascription to Logger's apply method #263

Closed mtkp closed 4 years ago

mtkp commented 4 years ago

Problem

when logging strings that sometimes contain "%" (e.g. URLs with escaped characters), if i also log an exception, i might get a java.util.UnknownFormatConversionException. this only happens with the Logger's apply method.

Solution

added a missing type ascription to the apply method, so it sends the correct items sequence to the log method.

Result

fixes added test case (using apply with strings that have %)

claassistantio commented 4 years ago

CLA assistant check
All committers have signed the CLA.

codecov-io commented 4 years ago

Codecov Report

Merging #263 into develop will increase coverage by 0.36%. The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #263      +/-   ##
===========================================
+ Coverage    45.87%   46.23%   +0.36%     
===========================================
  Files          228      228              
  Lines        14436    14622     +186     
  Branches       902      919      +17     
===========================================
+ Hits          6622     6761     +139     
- Misses        7814     7861      +47
Impacted Files Coverage Δ
...ng/src/main/scala/com/twitter/logging/Logger.scala 62.5% <100%> (+0.78%) :arrow_up:
util-core/src/main/scala/com/twitter/io/Buf.scala 92% <0%> (+0.21%) :arrow_up:
...core/src/main/scala/com/twitter/util/Promise.scala 79.11% <0%> (+0.4%) :arrow_up:
...-core/src/main/scala/com/twitter/util/Future.scala 54% <0%> (+5.7%) :arrow_up:
...in/scala/com/twitter/logging/QueueingHandler.scala 100% <0%> (+6.06%) :arrow_up:

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 0f2beaa...d7601f8. Read the comment docs.

bryce-anderson commented 4 years ago

Merged as c875d359d6505a6979258f9fd596fa05ebd3c792. Thanks!

mtkp commented 4 years ago

thanks @bryce-anderson !