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

Remove extra log about error encoding tag #2976

Merged
merged 1 commit into from
Jan 18, 2019
Merged

Conversation

nmuesch
Copy link
Collaborator

@nmuesch nmuesch commented Jan 18, 2019

What does this PR do?

_to_bytes is logging a message when it can't encode. However, the calling function of _to_bytes has more context and prints a more useful message.

Motivation

No need to log multiple times for the same error.

Review checklist

  • PR has a meaningful title or PR has the no-changelog label attached
  • Feature or bugfix has tests
  • Git history is clean
  • If PR impacts documentation, docs team has been notified or an issue has been opened on the documentation repo
  • If PR adds a configuration option, it has been added to the configuration file.

Additional Notes

Anything else we should know when reviewing?

@codecov-io
Copy link

codecov-io commented Jan 18, 2019

Codecov Report

Merging #2976 into master will decrease coverage by 9.45%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2976      +/-   ##
==========================================
- Coverage   84.91%   75.45%   -9.46%     
==========================================
  Files         656       46     -610     
  Lines       36072     3573   -32499     
  Branches     4276      427    -3849     
==========================================
- Hits        30629     2696   -27933     
+ Misses       4188      768    -3420     
+ Partials     1255      109    -1146

@nmuesch nmuesch mentioned this pull request Jan 18, 2019
5 tasks
@nmuesch nmuesch merged commit 578d737 into master Jan 18, 2019
@nmuesch nmuesch deleted the nick/remove_dupe_log branch January 18, 2019 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants