Skip to content
This repository has been archived by the owner on Jun 22, 2023. It is now read-only.

.NET Core Nuget package #20

Closed
RegiSV2 opened this issue Oct 19, 2016 · 31 comments
Closed

.NET Core Nuget package #20

RegiSV2 opened this issue Oct 19, 2016 · 31 comments

Comments

@RegiSV2
Copy link

RegiSV2 commented Oct 19, 2016

Hello.

There is already a working .NET Core project for GeoAPI. Are there any chances of releasing a nuget package?

@nickalbrecht
Copy link

I see there is a folder in the source for this, I'd love to get an official support for it in the Nuget package

@skarllot
Copy link
Contributor

I've created a pull request #21 which includes .NET Core library.

@HarelM
Copy link

HarelM commented Jan 3, 2017

Does version 1.7.4 has this support in it?

@xivk
Copy link
Contributor

xivk commented Jan 19, 2017

This issue is blocking this issue:

NetTopologySuite/NetTopologySuite#110

How can we more forward on this... anything I can do?

Once we have a GeoAPI package supporting netstandard we can move on with NTS. Do we also convert the netcore project here to VS2017 format, similar to what we decided for NTS.

@DGuidi
Copy link
Contributor

DGuidi commented Jan 20, 2017

To be fair, I haven't VS2017 and a lot of ignorance about .netcore and .netstandard.
So basically I'm unable to give any kind of support.
I'm reluctant because of the additional complexity we can add to project, but if you will be provide support (and for support I mean CI-Travis integration, "legacy" plain .NET compatibility, etc...) for this stuff, we can proceed.

@pauldendulk
Copy link
Member

I think it must be done. It is the only way forward. Perhaps the new nuget tooling can help. There is Nugetizer 3000 and perhaps Visual Studio 2017 has improved tooling. The free community edition is quite powerful.

@DGuidi
Copy link
Contributor

DGuidi commented Jan 20, 2017

I think it must be done.

No bad feelings: I'm sure it must be done and I'm sure can offer a lot of added value.
Simply, I hope someone with the necessary skills and knowledge will grab this task and get things done.

@xivk
Copy link
Contributor

xivk commented Jan 20, 2017

As I said before, I'm willing to put in the work but we have to agree more or less and I can't publish nugets.

I would say:

  • Move GeoAPI to the new format, VS 2017 community edition is easy and fast to install.
  • Adjust the build process, we should be able to just install the lastest netcore similar to what's there now.
  • Publish nugets asap.

I would also recommend publish nugets more often, I publish nugets straight from my build server... perhaps we could do that for GeoAPI/NTS too? I publish, obviously, only when tests are all green and according to branch:

  • master: always latest release, in synch with nuget releases
  • develop: always latest develop version, pushes a prerelease package on every commit.

It increases # of contributions and bugfixes because there is an immidiate reward. Only the core contributor group decides when to merge to master and publish a 'stable' release.

@DGuidi
Copy link
Contributor

DGuidi commented Jan 20, 2017

I'd like to have some suggestions from @FObermaier , that is actually the core maintainer of the project at this time. To me, I agree with all your suggestions.

I can't publish nugets

I'f it's only a problem of credentials in nuget website, I can share this credentials with you (if @FObermaier agree, too).

@HarelM
Copy link

HarelM commented Jan 20, 2017

I also have some time to invest in it if needed. I really want to try and migrate my site from .net 4.5 to .Net Core but for this to happen I need GeoApi, NTS, OSMSharp to be in .Net Core as well.

@FObermaier
Copy link
Member

FObermaier commented Jan 23, 2017

  1. I must admit that I'm not comfortable with targeting "NETStandard.Library": "1.6.0".
    This seems to be the fastest way to go, but is it the right way? Could we achieve a much broader coverage by investing time to let it target netStandard 1.0 or 1.1.
    NewtonSoft.Json does it that way.

  2. The state of the codebase in GeoAPI is a mess right now with all those folders for different project types but mainly empty.

That is why I'm reluctant to releasing the package as is. I've spend some time on the 1st point but am not finished yet.

@xivk
Copy link
Contributor

xivk commented Jan 23, 2017

netstandard 1.0 is already on it's way out and being replaced by netstandard 2.0 anyway, there aren't a lot of users using netstandard 1.0 and 2.0 will be supported almost everywhere. Supporting netstandard 1.6 will be the best option AFAIK and supporting 2.0 will be very easy once we've done this.

We need to move faster, I can't use NTS/GeoAPI in any of my latest projects or the libraries I publish, it's the one project holding me back and this issue shows there are others with the same problem. What about releasing pre-release packages? We can move a lot faster that way and keep the stable releases the same as they are now. We can test integration with other packages and thus even improve the project overall.

@HarelM
Copy link

HarelM commented Jan 23, 2017

I have installed and ran VS 2017 over the weekend, I think it's safe to say it continues the csproj line of thought from earlier versions and it's stable enough to work with (and a lot shorter and editable, which is a huge plus).
@xivk I think either you or me should create a pull request with the relevant changes and the guys here will review it, have rejects and ping-pong it until it gets to the required quality.
After that we can ask for a NuGet pre-release and continue to do the same for NTS.
Please let me know if you want me to do the initial PR or not.

@xivk
Copy link
Contributor

xivk commented Jan 24, 2017

@HarelM The issue isn't a project for netcore because there is already one in master, for a couple of months even, the issue is that we can't do the same in NTS because there is no nuget package for it yet.

@nickalbrecht
Copy link

It's worth noting that while VS2017 in in RC status, the .NET Core Tooling in it is actually only in alpha/preview if I recall. I am personally avoiding 2017 for .NET Core projects until the tooling catches up.

Also, I think targeting the earlier .NET Standards of 1.0 or 1.1 (similar to what was said above by @FObermaier) is the way to go (at least as a first step) unless something from later versions of .NET standard are actually needed. Keeping the target of .NET standard low for dependency assemblies like this is a much better option in my opinion. People who consume it can try for .NET Standard 2.0 if they want as I believe it's all backwards compatible. But if you target 1.6 or even 2.0 with NTS, you end up excluding support for a few platforms. From my understanding of the way .NET Standards works, you should be able to target .NET Standard and eliminate some of the other project folders/files (like PCL) in the source. Don't take my word for it, but it's worth looking into.

I would mention for those interested in migrating their existing websites, you can migrate your existing ASP.NET project to use ASP.NET Core and still target .NET 4.x in your code. The two are independant, and then you can migrate the code itself to .NET Core (or Standard) gradually. However, this fact is unrelated is unrelated to the issue.

@HarelM
Copy link

HarelM commented Jan 24, 2017

I'm not sure I agree with both your claims.
It's like saying this library should be published in .Net 2.0 (not core) in order to support all .Net application in all versions. No one is developing application in .Net 2.0 now. If the current stable version at the time of migration is 1.6 then this should be the target version IMHO.

The main advantage of migrating ASP.NET web application to .Net core IMO is the cross platform and docker support, and I prefer not to do the migration in two steps since I would need to make sure everything is working twice.
But that's just me.
In any case I can't find a reason not to publish a pre release NuGet package. The stable version can be different in terms of target version etc.

@DGuidi
Copy link
Contributor

DGuidi commented Jan 25, 2017

What about to be more conservative in the official branch and made a single specific fork that targets this kind of issues/improvement like .net standard support and pre-release packages?
Keep the two repo in sync should be easy, I hope.
Can be this a viable solution?

@xivk
Copy link
Contributor

xivk commented Jan 25, 2017

Ok, lets agree then to:

  • Target as low a netstandard version as possible.
  • use the branching scheme to release pre-release packages:
    • master: always latest stable, same as now.
    • develop: always published to nuget with an increasing build number as prerelease

I also think we should build a matrix with what platforms we want to support, support now and how long we want to keep supporting them.

xivk added a commit that referenced this issue Jan 25, 2017
@xivk
Copy link
Contributor

xivk commented Jan 25, 2017

Was pretty easy, netstandard1.0 just works so that's good...

@FObermaier
Copy link
Member

Must read

@nickalbrecht
Copy link

nickalbrecht commented Jan 25, 2017

@HarelM I think you're missing my point. Looking at the project files for NTS and GeoAPI, they are both using .NET 4.0 Client profile by default. The lowest .NET Standard supports is 4.5. So even with using 1.0 it's still an increase in supported version of .NET. By using .NET Standard 1.1 or 1.3 and higher, you start to drop off support for some platforms. All I'm trying to say is that unless there is something specifically gained through the increase in API coverage of the higher versions, I don't see what the benefit of targeting 1.6 or waiting until 2.0?
.NET Standard Platform Support

@DGuidi
Copy link
Contributor

DGuidi commented Jan 25, 2017 via email

@xivk
Copy link
Contributor

xivk commented Jan 26, 2017

I'm not following anymore, all I know is we now have GeoAPI (in the current state of the code) supporting:

  • .NET 2.0
  • .NET 3.5
  • .NET 3.5 CE
  • .NET 4.0
  • .NET 4.0.3
  • .NET 4.5
  • PCL profile portable-net403+sl5+netcore45+wpa81+wp8+MonoAndroid10+XamariniOS10+MonoTouch10
  • netstandard 1.0 (!)

What's holding us back? This is amazing, most of these platforms aren't even supported anymore and we are fully up to date with all the new stuff...

xivk added a commit that referenced this issue Jan 26, 2017
@DGuidi
Copy link
Contributor

DGuidi commented Jan 26, 2017

We're perfectly aligned, all platform you lister are the platform actually supported.
Probably (maybe?) PCL can be removed in favor of netstandard?

@xivk
Copy link
Contributor

xivk commented Jan 26, 2017

Think we need to keep PCL for a while longer. If we want to remove stuff I would say remove .NET 2.0 - .NET 4.5 because .NET 4.0.3 and higher are supported by the PCL anyway and 2.0 and 3.5 are really really getting old.

On the other hand, it's not difficult keeping them around for now. I now:

  • Fixed the build script for windows to build everything.
  • Updated the nuspec for netstandard1.0
  • Fixed the build script for netstandard 1.0 for travis.

I also setup an automatic build process on my own build server, published packages here:

http://public-nuget.itinero.tech/

@FObermaier
Copy link
Member

Thanks a lot for your effort @xivk .
I have tested the build.bat approach and it works fine. I have a couple questions.:

  • Is it save to rename then .net core output to GeoAPI.dll. If not I think we should put some more effort in having the output named GeoAPI.dll.
  • Do we have to include the *deps.json file in the package

@xivk
Copy link
Contributor

xivk commented Jan 30, 2017

Hmm, not sure about the *deps.json, I don't include this in my other packages. Why should this be included? I do realize now that the nuspecs aren't 100% correct anymore, will correct this soon.

And yes, I don't see any issue with renaming to GeoAPI.dll, will try and do that too...

@FObermaier
Copy link
Member

1.7.5 prerelease tagged and published on nuget:

@HarelM
Copy link

HarelM commented Jan 31, 2017

Great!!! when can we expect a NTS .Net core package pre-release?

@xivk
Copy link
Contributor

xivk commented Jan 31, 2017

@FObermaier 👍 Thanks a lot!

@HarelM Now I can get started on doing the same there...

@xivk
Copy link
Contributor

xivk commented Mar 22, 2017

Considering this closed for now, I still suggest cleaning up the project structure later this year.

@xivk xivk closed this as completed Mar 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants