-
-
Notifications
You must be signed in to change notification settings - Fork 424
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
Feat add candidate output #421
Feat add candidate output #421
Conversation
Codecov Report
@@ Coverage Diff @@
## master #421 +/- ##
==========================================
- Coverage 83.36% 83.17% -0.19%
==========================================
Files 68 68
Lines 5289 5302 +13
Branches 937 942 +5
==========================================
+ Hits 4409 4410 +1
- Misses 609 620 +11
- Partials 271 272 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@@ -215,7 +215,8 @@ def get_metadata( | |||
return self.metadata | |||
|
|||
def __repr__(self) -> str: | |||
return f"<Candidate {self.name} {self.version}>" | |||
source = getattr(self.link, "comes_from", "unknown") |
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.
What is the value if the candidate is from a file link(including VCS, remote file and local file)?
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.
Good question. I haven't tried and don't know. At least it doesn't break if comes_from
is not set.
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.
Just to make sure it displays a meaningful value not confusing
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.
For the cases you mentioned, pdm doesn't seem to go through the repository search. At least I wasn't able to get the candidate list output.
The UI needs more careful design, consider this:
And in fact, there is a redundant output level |
If you give me your preferences on those topics, I'm happy to adapt the code. I also thought about using the |
Here is a output with the change of this PR: As you can see there are lots of duplicate lines that are not informative. I recommend only log failing cases here in |
You can log the unfiltered candidate list (length limited to top 10) when |
I don't understand the "lacks color emphasis" in context with logging the output to a file: We cannot use color there, right? |
You are right, I turned verbose on so the output goes to stdout, improving the format is also good |
I'm going to filter out the duplicate lines. Please don't accept yet. |
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.
LGTM 💯
Thanks for contribution |
Pull Request Check List
news/
describing what is new.Describe what you have changed in this PR.
I'm currently struggling with installing packages from multiple repos and had the impression that the log output could be improved.
The PR adds some more log output (log level: DETAIL) about matching candidates and their origin. If no matching candidates could be found, the non-matching candidates are printed to give the user some idea what might have gone wrong.