-
Notifications
You must be signed in to change notification settings - Fork 527
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
Fixes an issue where the binding redirect version is picked from the fir... #422
Conversation
…first file found rather than the highest package version.
@@ -104,7 +111,8 @@ let private applyBindingRedirects root extractedPackages = | |||
match ref with | |||
| Reference.Library path -> Some path | |||
| _-> None) | |||
|> Seq.distinctBy (fun p -> FileInfo(p).Name) | |||
|> Seq.groupBy (fun p -> FileInfo(p).Name) | |||
|> Seq.map(fun (_,ps) -> pickHighestPackageVersion ps) |
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.
I think the issue is that we're not talking about packages here.
We have libs with conflicting version nos.
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.
I just followed from the extractedPackages and made the assumption p = package
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.
yep sorry that's my fault. Should have made it more explicit. p is a library path in this case.
Could you please fix this with your PR? Thanks
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.
all of it or just my changes?
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.
No it's just in the lines you touched.
@isaacabraham could you please review this one? Thanks |
@@ -95,6 +96,12 @@ let createModel(root, sources,force, lockFile:LockFile) = | |||
|
|||
extractedPackages | |||
|
|||
/// Picks the highest version of a library | |||
let private pickHighestlibraryVersion libraries = |
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.
Can you adjust the casing of library
to Library
.
Fixes an issue where the binding redirect version is picked from the fir...
Thanks for fixing this. |
...st file found rather than the highest package version.
This fixes was the issue I had with #421