koodaamo / tnefparse

a TNEF decoding library written in python, without external dependencies
GNU Lesser General Public License v3.0
49 stars 37 forks source link

Use % formatting in log messages #86

Closed eumiro closed 3 years ago

eumiro commented 3 years ago

Replaced 'hello %s' % world and f'hello {world}' syntax with the old standard lazy 'hello %s', world in all log messages.

codecov[bot] commented 3 years ago

Codecov Report

Merging #86 (72b5fce) into master (a9668ef) will not change coverage. The diff coverage is 16.66%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #86   +/-   ##
=======================================
  Coverage   92.90%   92.90%           
=======================================
  Files           7        7           
  Lines        1296     1296           
  Branches      117      117           
=======================================
  Hits         1204     1204           
  Misses         70       70           
  Partials       22       22           
Impacted Files Coverage Δ
tnefparse/mapi.py 80.00% <0.00%> (ø)
tnefparse/util.py 66.66% <0.00%> (ø)
tnefparse/tnef.py 92.06% <25.00%> (ø)

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 a9668ef...72b5fce. Read the comment docs.

petri commented 3 years ago

Why should we use % formatting for log messages? I seem to remember that for logging, recommended practice is to pass variables to format string as we do now. See e.g. https://reinout.vanrees.org/weblog/2015/06/05/logging-formatting.html . Some other reason why % formatting is better in this case? Something changed in Python?

eumiro commented 3 years ago

Sorry, my title is a little bit misleading, but the proposed code reflects my intentions. I meant using % within the template strings and pass the parameters after commas.

Both 'hello %s' % 'world' and f'hello {world}' build the whole string whether the logging level is needed or not. I am proposing to use the old standard 'hello %s', world syntax that builds the string only if needed.

Just like it is described in the article you linked.