-
Notifications
You must be signed in to change notification settings - Fork 288
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
Split up FSharp.Data #1457
Split up FSharp.Data #1457
Conversation
What's the benefit to pulling it out to be a separate dll? |
@cartermp The relevant issue is here: #1311 We can decide on the split we want. To me the following eventually makes sense, as it's reasonable to just want the helpers for a particular data format:
What's currently in this PR is this, which is a stepping stone to the above:
I don't think it's worth splitting the different type providers out as yet - at least it's a lot of work as each also needs a DesignTime thing. But we should probably leave the door open to doing that. |
Maybe a better alternative is this:
This is based on the logic that
That does mean splitting the specific type provider DesignTime out, it's work though the user doesn't see that. A stepping stone could be:
|
@dsyme makes sense! |
@@ -141,15 +141,16 @@ Target.create "NuGet" (fun _ -> | |||
("PublishRepositoryUrl", "true") | |||
("EmbedUntrackedSources", "true") | |||
("IncludeSymbols", "true") | |||
("SymbolPackageFormat", "snupkg") ] |
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.
Why is this commented out?
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.
So I can't work out why having this in fails the package build when there are two packages.
Without it we get FSharp.Data.Core.symbols.nupkg
and FSharp.Data.symbols.nupkg
correctly built but the file name extensions are different
Do you know if it matters either way?
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.
From https://docs.microsoft.com/en-us/nuget/create-packages/symbol-packages-snupkg
The legacy format .symbols.nupkg is still supported but only for compatibility reasons like native packages (see Legacy Symbol Packages). NuGet.org's symbol server only accepts the new symbol package format - .snupkg.
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 just embed symbols instead of the separate package? That's much less hassle for consumers. Separate packages make Sourcelink hard to use.
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.
@baronfel Thanks, I've done this now
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.
Thank you!
I've updated the PR to split out the core pacakges |
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.
Looks clean to me (pending docs changes)
This is now merged and released as FSharp.Data 5.0.1 |
@cartermp We've discussed splitting out core packages for Http, Csv etc.
The split in this PR is this:
At a later point we may split out the separate type providers to allow you to use just the one you want. However that's not done here - currently if you want a type provider you reference FSharp.Data which implies all the core pacakges too.
Some of the design-time inference logic used by the type providers is in the Core packages, now made internal.
Some of the public IO and pluralization helpers used by the the other components and the type provider inference are in FSharp.Data.Http. These may be moved around or made internal in a future release. There's no "right" place to put these.