Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove Pool from API #40

Merged
merged 4 commits into from
Jun 7, 2024

Conversation

baszalmstra
Copy link
Contributor

@baszalmstra baszalmstra commented Jun 4, 2024

The Pool object implements interning but the solver doesn't care about that at all. It just needs to be able to refer to IDs and be able to call functions on the DependencyProvider using those IDs. This PR refactors the code to remove any reference of the Pool from the Solver. Instead, additional methods are added to the DependencyProvider to convert the IDs (which the DependencyProvider already provides), to strings and to filter a list of solvables with a VersionSetId (previously implemented in the VersionSet trait). The DependencyProvider trait now encapsulates everything required to implement a packaging ecosystem without requiring the use of a Pool. 📦 🍿

This should make it easier to create bindings to other languages because the actual datatypes used don't even need to be stored inside resolvo itself. They can be kept completely externally as long as the code can provide the proper Ids.

I was also able to remove loads of trait bounds through this refactor!

The Pool is still a useful tool for interning purposes so I kept it. Both the tests and the implementation in rattler most likely still benefit from it.

This is a pretty significant refactor, which I would be happy to explain offline. 😄

TODO:

  • Use the new API in rattler to see if anything is missing.

@baszalmstra baszalmstra requested review from tdejager and wolfv June 4, 2024 21:19
@baszalmstra baszalmstra changed the title refactor: simplify solvableid refactor: remove Pool Jun 4, 2024
@baszalmstra baszalmstra force-pushed the refactor/intern_dependency_provider branch from 01416cf to 3174889 Compare June 4, 2024 21:27
@baszalmstra baszalmstra changed the title refactor: remove Pool refactor: remove Pool from API Jun 4, 2024
@baszalmstra baszalmstra changed the title refactor: remove Pool from API refactor: remove Pool from API Jun 4, 2024
@baszalmstra baszalmstra force-pushed the refactor/intern_dependency_provider branch from 3174889 to 204cab5 Compare June 4, 2024 21:29
@baszalmstra baszalmstra force-pushed the refactor/intern_dependency_provider branch from 204cab5 to 4e0ac96 Compare June 4, 2024 21:31
@baszalmstra baszalmstra mentioned this pull request Jun 6, 2024
@baszalmstra baszalmstra marked this pull request as ready for review June 7, 2024 13:41
@baszalmstra baszalmstra merged commit d83abfb into main Jun 7, 2024
3 checks passed
@baszalmstra baszalmstra deleted the refactor/intern_dependency_provider branch June 7, 2024 14:02
@baszalmstra baszalmstra mentioned this pull request Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants