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

Move logging configuration to main(). #45

Merged
merged 1 commit into from Mar 30, 2016

Conversation

pkaleta
Copy link
Contributor

@pkaleta pkaleta commented Mar 29, 2016

The problem with the current code is that it fails w/ the following message when diff-cover tool is executed after fresh installation:

% diff-cover coverage/coverage.xml
No handlers could be found for logger "diff_cover.tool"

The reason is that the default logger is configured at the module level, when the diff_cover.tool is executed as a standalone script:

if __name__ == "__main__":
    logging.basicConfig(format='%(message)s')
    exit(main())

However the script that is present in bin after installation using pip install diff-cover==0.9.6 imports the main function, which doesn't contain the basicConfig call.

Also, in #42, it was suggested that basicConfig call is moved to the main.

Alternative would be to perform logger configuration in bin/diff-cover, if there's a reason not to place it in main?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.988% when pulling 7341031 on pkaleta:master into ebc4239 on Bachmann1234:master.

@pkaleta
Copy link
Contributor Author

pkaleta commented Mar 29, 2016

@Bachmann1234 Care to comment why this has been refused? I'd really like to get the underlying issue fixed.

@Bachmann1234
Copy link
Owner

Oh crap. Think I hit a button. I'll fix that

@Bachmann1234 Bachmann1234 reopened this Mar 29, 2016
@Bachmann1234
Copy link
Owner

Sorry about that. Yeah, I'll mere and release tonight

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 99.036% when pulling 7341031 on pkaleta:master into ebc4239 on Bachmann1234:master.

@Bachmann1234 Bachmann1234 merged commit f86e33c into Bachmann1234:master Mar 30, 2016
@pkaleta
Copy link
Contributor Author

pkaleta commented Mar 30, 2016

Thanks!

@Bachmann1234
Copy link
Owner

Released! https://pypi.python.org/pypi/diff_cover/0.9.7 Thanks for the PR and sorry for the early confusion

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

3 participants