fix: decouple downloading of sources from resolution of package source #384
Conversation
This file contains hidden or 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
2 tasks
shubhbapna
force-pushed
the
decouple-download-source
branch
from
e381f74 to
4e6406f
Compare
dhellmann
approved these changes
Sep 4, 2024
Member
dhellmann
left a comment
dhellmann
left a comment
There was a problem hiding this comment.
A couple of nits for things to consider as follow-ups, but this looks good. Nice work with such a complicated refactor!
| evaluated, version of the package being downloaded and the URL | ||
| from which the source can be downloaded as returned by `resolve_source`. | ||
|
|
||
| The return value should be a `pathlib.Path` file path to the downloaded source. |
Member
There was a problem hiding this comment.
Maybe we should also pass the path to the directory where we expect the file to be written? The plugin can get it from the context, but it's one less thing a plugin author has to know. Something to think about for a follow-up.
| `sources.resolve_dist()`, which will call `get_resolver_provider()`, | ||
| and then the return value of the resolution will be passed back to | ||
| `download_source()` as a tuple of URL and version. The provider can | ||
| The expectation is that it acts an engine for any sort of package resolution |
Member
There was a problem hiding this comment.
Suggested change
| The expectation is that it acts an engine for any sort of package resolution | |
| The expectation is that it acts as an engine for any sort of package resolution |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
part of #368
I will update the fields in settings in a separate PR