-
Notifications
You must be signed in to change notification settings - Fork 150
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
Only configure setuptools logging if bdist_wheel is imported #641
Conversation
This also aims to improve the correctness of the egg and bdist_wininst conversions.
Also fix the output of `wheel convert` to add the final "OK" on the same line as the source file name. Fixes #632.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #641 +/- ##
==========================================
+ Coverage 77.96% 78.14% +0.17%
==========================================
Files 14 14
Lines 1121 1121
==========================================
+ Hits 874 876 +2
+ Misses 247 245 -2 ☔ View full report in Codecov by Sentry. |
@@ -34,6 +34,15 @@ | |||
if TYPE_CHECKING: | |||
import types | |||
|
|||
# ensure Python logging is configured | |||
try: | |||
__import__("setuptools.logging") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__import__("setuptools.logging") | |
import setuptools.logging |
Why not this? They should be the same, right? Usually you use this form if you have a variable value to import.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Though now I see this is just moved...)
@@ -293,7 +293,7 @@ def convert(files: list[str], dest_dir: str, verbose: bool) -> None: | |||
source = WininstFileSource(path) | |||
|
|||
if verbose: | |||
print(f"{archive}... ", flush=True) | |||
print(f"{archive}...", flush=True, end="") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be to stderr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so...? There's no docs indicating either way, and wheel tags
prints the file names to stdout.
This prevents the CLI commands from emitting unwanted output.
Fixes #632.