-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Simplify Resolution
type
#5301
Simplify Resolution
type
#5301
Conversation
/// The resolution from one or more forks including the virtual packages and the edges between them. | ||
/// | ||
/// Each package can have multiple versions and each edge between two packages can have multiple | ||
/// version specifiers to support diverging versions and requirements in different forks. | ||
#[derive(Debug, Default)] | ||
pub(crate) struct Resolution { | ||
pub(crate) packages: FxHashMap<ResolutionPackage, FxHashSet<Version>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we rename this field to nodes
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E.g. "nodes and edges" rather than "packages and edges"
f4ba2d3
to
0a08734
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, yeah, I think this is simpler. I wonder if our access patterns changed since I originally wrote this type.
Looking at how to merge identical forks, i found that the
Resolution
can be simplified by treating it as a nodes and edges store (plus pins, they are separate since they are per name-version, not per (virtual-)package-version). This should also make #5294 more apparent, which i didn't touch here.I additionally added some doc comments to the
Resolution
types.