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

Target framework: Added >= and made warnings more clear #768

Merged
merged 3 commits into from
Apr 10, 2015

Conversation

Thorium
Copy link
Member

@Thorium Thorium commented Apr 10, 2015

No description provided.

@forki
Copy link
Member

forki commented Apr 10, 2015

/cc @theimowski

@@ -184,7 +184,8 @@ module NugetEnv =
Packages = [for node in doc.SelectNodes("//package") ->
{ Id = node.Attributes.["id"].Value
Version = node.Attributes.["version"].Value |> SemVer.Parse
TargetFramework = node |> getAttribute "targetFramework" } ]}
TargetFramework =
node |> getAttribute "targetFramework" |> Option.map (fun t -> ">= " + t) } ]}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this is actually the only change necessary? i.e. we don't need to call the optimizeRestrictions function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I think the optimization happens later anyways. :-)

@forki
Copy link
Member

forki commented Apr 10, 2015

ready to merge?

@theimowski
Copy link
Member

I guess there's this one thing that could be corrected, from my last code comment - everything else seems fine

@Thorium
Copy link
Member Author

Thorium commented Apr 10, 2015

Fixed that, and some code clean-up also.

@forki
Copy link
Member

forki commented Apr 10, 2015

❤️

forki added a commit that referenced this pull request Apr 10, 2015
Target framework: Added >= and made warnings more clear
@forki forki merged commit cd3cc85 into fsprojects:master Apr 10, 2015
@theimowski
Copy link
Member

👍

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.

3 participants