feat!: associated types instead of generics for P and VS

pubgrub-rs / pubgrub

PubGrub version solving algorithm implemented in Rust

https://pubgrub-rs.github.io/pubgrub/pubgrub/

Mozilla Public License 2.0

337 stars 29 forks source link

feat!: associated types instead of generics for P and VS #190

Closed Eh2406 closed 3 months ago

Eh2406 commented 3 months ago

Dependency provider was inconsistent about its types, P and VS were generics but Err and Priority were associated types. If/as we had more configuration the benefits of associated types become even more valuable. This PR goes to the breaking change of making everything consistently associated types.

This was in preparation for adding user controlled metadata for UnavailableDependencies. But this was sufficiently complicated to deserve its own PR before we get into the additional complications of that extension.

Eh2406 commented 3 months ago

@zanieb and @mpizenberg what are you thinking about the resent pushes?

mpizenberg commented 3 months ago

Recent commits only add some docs right? or am I missing something. I haven't looked at it again since last week when commenting.

Eh2406 commented 3 months ago

No.

mabe V as well? adds an associated type for V on DP.
more source uses the new + 'static by marking more errors as sources.

zanieb commented 3 months ago

They seem reasonable to me but frankly generics in Rust APIs are not something I have a lot of experience with yet. I'd be curious to see the metadata pull request stacked on top of this one, I can try to integrate in that into uv and we can see what it looks like in-use?

mpizenberg commented 3 months ago

Ah yes, I like the new V. As for #[source] in errors, honestly I haven't coded in rust for so long that I don't know what happens there ^^.

Eh2406 commented 3 months ago

If we all like the basic direction this is going, let's merge. There will be more breaking changes before we cut 0.3, if we find problems or change our minds we can fix it then.

I'd be curious to see the metadata pull request stacked on top of this one, I can try to integrate in that into uv and we can see what it looks like in-use?

That does seem like the next step, and I'm happy to have you take the first stab at it. Something like add a type Metadat: Eq + Clone + Debug + Display to DependencyProvider and use it in Kind and External so that it can be passed in with a Incompatibility and read as part of the DerivationTree. If that seeming overwhelming, start with something that useful for you and we can generalize from there.

zanieb commented 3 months ago

Sounds good to me!

Eh2406 commented 3 months ago

If someone could officially give a approving review, we can get this merged.