-
Notifications
You must be signed in to change notification settings - Fork 760
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 installer to InstalledDist
#1762
Conversation
@@ -111,6 +111,15 @@ impl InstalledDist { | |||
}) | |||
} | |||
|
|||
/// Return the `INSTALLER` of the distribution. | |||
pub fn installer(&self) -> Option<String> { |
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.
Do you mind making this Result<Option<String>>
, and erroring if the error kind is anything other than "not found"?
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 feel like the direct_url
method above should function similarly, but that doesn't need to be changed here.)
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.
Gonna change this so that I can include it in the release.
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.
Sure thanks!
Summary
Add
installer
method toInstalledDist
to distinguish between different installers. Might be nice to add an enum for all possible installers, but this might be too hard to keep up to date :).The
INSTALLER
file is a file that can be added to the.dist-info
folder with the installer name.Closes: #1759
Test Plan
Not sure if there is a place I can automatically test it, if you have a pointer I would be happy to add a test.