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

Add context to the "Discarded Event" warning #745

Merged
merged 2 commits into from
May 23, 2021

Conversation

dd32
Copy link
Contributor

@dd32 dd32 commented May 23, 2021

The Discarding %s event warning is logged with the %s left as is, instead of replacing the expected context of which event is being discarded.

The `Discarding %s event` warning is logged with the `%s` left as is, instead of replacing the expected context of which event is being discarded.
@dd32 dd32 marked this pull request as draft May 23, 2021 03:32
@coveralls
Copy link

coveralls commented May 23, 2021

Coverage Status

Coverage remained the same at 99.846% when pulling ac98bdc on dd32:fix/missing-placeholder into 103ac00 on zigpy:dev.

@codecov-commenter
Copy link

codecov-commenter commented May 23, 2021

Codecov Report

Merging #745 (ac98bdc) into dev (103ac00) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #745   +/-   ##
=======================================
  Coverage   99.84%   99.84%           
=======================================
  Files          43       43           
  Lines        5844     5844           
=======================================
  Hits         5835     5835           
  Misses          9        9           
Impacted Files Coverage Δ
zigpy/appdb.py 98.42% <100.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 103ac00...ac98bdc. Read the comment docs.

@puddly
Copy link
Collaborator

puddly commented May 23, 2021

Thanks for the PR! It looks like Black is formatting the call to LOGGER.warning to take up three lines due to the trailing comma in the current zigpy codebase. Adding the second missing argument makes it like a "normal" function call and it can be condensed into a single line:

            LOGGER.warning("Discarding %s event", cb_name)

@Adminiuga Adminiuga marked this pull request as ready for review May 23, 2021 17:12
@Adminiuga Adminiuga merged commit 5d58175 into zigpy:dev May 23, 2021
@dd32
Copy link
Contributor Author

dd32 commented May 24, 2021

Thanks for the quick response and following that through @puddly and @Adminiuga!

I came back to follow up on the contributing guidelines & unit test failure only to find it was already done :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants