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

Change log level of hash message #1460

Merged
merged 3 commits into from
Aug 2, 2021
Merged

Conversation

plannigan
Copy link
Contributor

@plannigan plannigan commented Aug 1, 2021

Minor change. It seemed odd that this was the only log message in the entire code bash using log() directly. As a result, it was not effected by the verbosity level.

I'm open to using a different log level. I went with info() as it would not change the currently default behavior.

Changelog-friendly one-liner: Adjust log level for PyPI hash message.

Contributor checklist
  • Provided the tests for the changes.
  • Assure PR title is short, clear, and good to be included in the user-oriented changelog
  • Assure one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

Copy link
Member

@atugushev atugushev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! I'd prefer debug though:

piptools/repositories/pypi.py Outdated Show resolved Hide resolved
@atugushev atugushev added bug fix logging Related to log or console output labels Aug 2, 2021
Co-authored-by: Albert Tugushev <albert@tugushev.ru>
@plannigan plannigan requested a review from atugushev August 2, 2021 11:44
Copy link
Member

@atugushev atugushev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@plannigan
Copy link
Contributor Author

There were a few test cases that explicitly checked for the log message. With the move to debug, they were failing because it was no longer being written. I updated the test cases to check for no message being written as it is the smallest change. A different option would to to remove the output check, as I don't think explicitly checking for log messages provide much value.

@atugushev atugushev added this to the 6.3.0 milestone Aug 2, 2021
@atugushev atugushev merged commit 14afbf8 into jazzband:master Aug 2, 2021
@plannigan plannigan deleted the hash-logging branch August 2, 2021 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
logging Related to log or console output
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants