Make Directory its own distribution kind
#3519
Conversation
charliermarsh
added
the
internal
| @@ -71,6 +72,7 @@ impl<'a> SourceUrl<'a> { | |||
| Self::Direct(dist) => dist.url, | |||
| Self::Git(dist) => dist.url, | |||
| Self::Path(dist) => dist.url, | |||
| Self::Directory(dist) => dist.url, | |||
There was a problem hiding this comment.
I kind of wish the names were like... Directory and Archive? But Archive is a bit strange.
There was a problem hiding this comment.
I like Archive, pip uses that terminology too
There was a problem hiding this comment.
Is Archive in this context referring to a specific file like a .tar.gz? If that's always the case, then I like the name Archive.
There was a problem hiding this comment.
Yeah it needs to be a .tar.gz or .zip or .tar.bz2 or a few other extensions that we support.
There was a problem hiding this comment.
I don't know that it's a perfect name because (e.g.) when you provide Direct (i.e., a direct URL), that has to be a direct URL to... an archive. So they're both archives?
There was a problem hiding this comment.
Honestly Self::File could be the right name.
There was a problem hiding this comment.
Yeah that's a good point. I suppose PathArchive would also work and is maybe a bit more descriptive. But I agree that File also works. And it's more succinct.
| BuildableSource::Dist(SourceDist::Path(dist)) => { | ||
| if dist.path.is_dir() { |
There was a problem hiding this comment.
This is good IMO, we removed this branch in favor of types that encode the difference.
|
Need to fix test failures. |
charliermarsh
force-pushed
the
charlie/source-tree
branch
3 times, most recently
from
f20d3b5
to
1f1e274
Compare
charliermarsh
force-pushed
the
charlie/source-tree
branch
4 times, most recently
from
f418142
to
c940843
Compare
charliermarsh
force-pushed
the
charlie/source-tree
branch
from
c940843
to
b61b014
Compare
|
Ok, tests passing... |
| .as_ref() | ||
| .join("pyproject.toml") | ||
| .metadata() | ||
| .ok() |
There was a problem hiding this comment.
Should we raise those if they aren't file not found errors?
There was a problem hiding this comment.
Yes, I'll do it separately though since this is just a move from above.
| @@ -71,6 +72,7 @@ impl<'a> SourceUrl<'a> { | |||
| Self::Direct(dist) => dist.url, | |||
| Self::Git(dist) => dist.url, | |||
| Self::Path(dist) => dist.url, | |||
| Self::Directory(dist) => dist.url, | |||
There was a problem hiding this comment.
Is Archive in this context referring to a specific file like a .tar.gz? If that's always the case, then I like the name Archive.
| BuildableSource::Dist(SourceDist::Path(dist)) => { | ||
| if dist.path.is_dir() { |
Summary
I think this is overall good change because it explicitly encodes (in the type system) something that was previously implicit. I'm not a huge fan of the names here, open to input.
It covers some of #3506 but I don't think it closes it.