search

MobilityData / gtfs-realtime-bindings

Language bindings generated from the GTFS Realtime protocol buffer spec for popular languages.
Apache License 2.0
370 stars 127 forks source link

dotnet: Update to .NET 8 and most recent protobuf #131

Closed HopXXII closed 2 months ago

HopXXII commented 8 months ago

Created .NET 8 version which is the most recent version of .NET Core (#56) and a LTS successor of .NET 6 (#127).

jameslinjl commented 8 months ago

@bdferris-v2 Are you able to take a look at this? I haven't had any bandwidth to work on GTFS stuff lately :(

bdferris-v2 commented 8 months ago

Two questions:

1) Your PR seemingly shows diffs for many of the text files (README.md, UPDATING.md, .gitignore) where the entire file is marked as a diff, but the contents don't look different. This is making it hard to tell if there were any small content changes to these files. Did something like the line-ending change on these files? Any chance you could clean this up?

2) It's been a long time I've done anything with .NET, so forgive the dumb question. Is there any downside to targeting .NET 8 as opposed to .NET 6? I understand that 8 is the LTS successor for 6, but will upgrading to 8 now limit applications that want to use this library that are still on 6?

EnessenE commented 6 months ago

I am a bit late in this discussion, but I would like to butt in.

I would recommend targetting neither .NET 6 or .NET 8 but targetting .NET Standard 2.0 (or .1) instead. Targetting this gives the widest available range of compatibility as seen here. https://learn.microsoft.com/en-us/dotnet/standard/net-standard?tabs=net-standard-2-0

Targetting specific versions should only be done if you are going for a feature that is only available in that specific target framework.

EnessenE commented 6 months ago

I ended up doing that one for comparison. Converted to .NET standard 2.0 and the new "SDK" style for .csproj's. Kept is as a draft to not overshadow this PR (didnt upgrade protobuf for example)

scottpidzarko commented 3 months ago

I like @EnessenE's draft PR - it's simple and to the point, and having it be a net standard library just makes sense for a library of this purpose. It's been nearly 3 months without any discussion about this and I would love to have a brand new application I am working on be .NET 8 instead of .NET (framework) 4.8.

What would be holding back bringing this PR for net standard 2.0 out of draft, rejecting the other one that has the aforementioned issues, and approving the PR for net standard 2.0?

HopXXII commented 2 months ago

First, I apologize for my long inactivity. To answer @bdferris-v2:

  1. Sure, silly mistake, I'll fix it.
  2. It makes more sense to me to make the .NET Standard version, as others write. I didn't think of it before, but compatibility will be the greatest there, and that's actually what we want.

I'll try to work it in by the end of the week.

HopXXII commented 2 months ago

I think it's ready for review, @bdferris-v2.

scottpidzarko commented 2 months ago

Thanks for the motion on this, I really appreciate it :)

EnessenE commented 1 month ago

Small afterwards note: would be nice if someone pushed this to nuget ;)

HopXXII commented 1 month ago

Small afterwards note: would be nice if someone pushed this to nuget ;)

@bdferris-v2 is one of the owners in the NuGet gallery :)