feat: add first part of rattler_pypi_interop crate
#1151
Conversation
There was a problem hiding this comment.
Copilot reviewed 114 out of 122 changed files in this pull request and generated no comments.
Files not reviewed (8)
- crates/rattler_pypi_interop/test-data/find_distributions/Lib/site-packages/Flask-1.1.4.dist-info/INSTALLER: Language not supported
- crates/rattler_pypi_interop/test-data/find_distributions/Lib/site-packages/Flask-1.1.4.dist-info/LICENSE.rst: Language not supported
- crates/rattler_pypi_interop/test-data/find_distributions/Lib/site-packages/Flask-1.1.4.dist-info/METADATA: Language not supported
- crates/rattler_pypi_interop/test-data/find_distributions/Lib/site-packages/Flask-1.1.4.dist-info/RECORD: Language not supported
- crates/rattler_pypi_interop/test-data/find_distributions/Lib/site-packages/Flask-1.1.4.dist-info/WHEEL: Language not supported
- crates/rattler_pypi_interop/test-data/find_distributions/Lib/site-packages/Jinja2-2.11.3.dist-info/INSTALLER: Language not supported
- crates/rattler_pypi_interop/test-data/find_distributions/Lib/site-packages/Jinja2-2.11.3.dist-info/LICENSE.rst: Language not supported
- crates/rattler_pypi_interop/test-data/find_distributions/L 8000 ib/site-packages/Jinja2-2.11.3.dist-info/METADATA: Language not supported
|
Just want to let you know that this is on my radar! I will take a look on Thursday! |
There was a problem hiding this comment.
Sorry for the late review.
I think there are a few types that can be replaced by pep508_rs or other crates. The less we have to maintain here the better.
There is a lot of test-data that is unused. Lets get rid of that.
Lets also remove commented out unused code.
| @@ -0,0 +1,180 @@ | |||
| use miette::Diagnostic; | |||
There was a problem hiding this comment.
I dont think we need this type. Everthing we would need is included in pep508_rs::PackageName
| // Private modules | ||
| // mod utils; | ||
|
|
||
| // Public modules | ||
| // pub mod artifacts; | ||
| // pub mod index; | ||
| // pub mod python_env; |
There was a problem hiding this comment.
Lets remove any code that is not used.
| //! | ||
| //! - Querying a `PyPI` repository for package metadata. | ||
| //! - Reading Python package metadata from locally installed packages. | ||
| //! |
There was a problem hiding this comment.
I think it would be especially good to amend this with what the library wont do!
| native-tls = ['reqwest/native-tls'] | ||
| rustls-tls = ['reqwest/rustls-tls'] | ||
|
|
||
| [dependencies] |
There was a problem hiding this comment.
Are all these dependencies actually used? It seems like a lot for just the types? You can check with cargo machete
| @@ -0,0 +1,119 @@ | |||
| // Implementation comes from https://github.com/njsmith/posy/blob/main/src/vocab/extra.rs | |||
There was a problem hiding this comment.
Lets use https://docs.rs/pep508_rs/latest/pep508_rs/struct.ExtraName.html instead.
| pub type Fields = HashMap<String, Vec<String>>; | ||
|
|
||
| #[cfg_attr(test, derive(Debug, serde::Deserialize, PartialEq, Eq))] | ||
| pub struct RFC822ish { |
There was a problem hiding this comment.
I think uv uses mailparse for this. Id much rather use an existing crate than maintain this ourselves.
There was a problem hiding this comment.
I think we dont use these sdist test data files do we?
There was a problem hiding this comment.
I dont think we use this.
There was a problem hiding this comment.
I dont think we use this vendored library!
Description
This pull request is an attempt to make initial pull request submitted (#1054) a little more manageable. It contains just the
typesmodule and this module has been trimmed down to remove sub-modules that depended on other modules in this crate. The missing files are:artifactartifact_nameinstall_pathproject_infoThe above will be add back in future pull requests.