Refactor metadata directory handling #7087
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pradyunsg
added
type: refactor
pradyunsg
force-pushed
the
refactor/metadata-directory
branch
from
202780a to
c4076ea
Compare
Why: Since the result is a single directory and it's better for the resposibility of computing the exact location of the metadata directory should be with the generator, that generates it.
Why: expecting metadata_directory to exist, once metadata is generated, is easier to reason about, than considering that some kinds of distributions set it while others do not.
Why: Changes made are IMO much clearer, with a step by step flow to this method that is easier to reason about.
pradyunsg
force-pushed
the
refactor/metadata-directory
branch
from
c4076ea to
ec5e9d3
Compare
pradyunsg
commented
Sep 28, 2019
| @@ -56,7 +62,10 @@ def _generate_metadata_legacy(install_req): | |||
| command_desc='python setup.py egg_info', | |||
| ) | |||
|
|
|||
| # Return the metadata directory. | |||
| return install_req.find_egg_info() | |||
There was a problem hiding this comment.
A follow up PR will move this method's contents into this file.
This was referenced Sep 28, 2019
pfmoore
approved these changes
Sep 28, 2019
|
Thanks a lot for taking a look @pfmoore! ^>^ |
|
I'm feeling good today, so I'm gonna go ahead and merge this since CI is happy and a different set of 👀 reviewed and did not spot any major concerns. :) |
lock
bot
added
the
auto-locked
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Built on top of #7086
Closes #7088
With this PR,
InstallRequirement.metadata_directoryrefers to the directory containing metadata for that object, regardless of how it was built (PEP 517 or legacy). If a metadata generator has been called on that object, themetadata_directorywould be set appropriately. This is IMO, much easier to reason about than status quo. (please don't ask me to explain how this worked in v19.2, thanks)InstallRequirement.get_dist()works, since some of the variables it modified, have been removed.