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 .NET Standard 2.1, .NET 6, .NET 8 only #8184

Merged
merged 9 commits into from
Dec 19, 2023

Conversation

bjornharrtell
Copy link
Collaborator

No description provided.

@github-actions github-actions bot added c# CI Continuous Integration documentation Documentation labels Dec 19, 2023
@dbaileychess
Copy link
Collaborator

I like that we are moving forward to support the latest frameworks, but I do want to make sure we are not losing support. We support .NET Standard 2.0 and Framework 46 on nuget: https://www.nuget.org/packages/Google.FlatBuffers

@bjornharrtell
Copy link
Collaborator Author

@dbaileychess the problem is for how long do we support? Indefinitely is difficult. Will be even harder for new versions of .NET which has shorter support cycles than the older .NET Framework (which is difficult by itself since it is windows only).

@dbaileychess
Copy link
Collaborator

OK, thanks for improving it.

@dbaileychess dbaileychess merged commit 129ef42 into google:master Dec 19, 2023
46 checks passed
@bjornharrtell bjornharrtell deleted the net-modernization branch December 20, 2023 08:04
@bjornharrtell
Copy link
Collaborator Author

@dbaileychess FYI for Unity this means at least version 2021.2 AFAIK as of this change.

candysonya pushed a commit to candysonya/flatbuffers that referenced this pull request Jan 8, 2024
* Target .NET Standard 2.1, .NET 6, .NET 8 only

* Remove mono usage

* Fix bat name ref

* Up deps

* Up deps

* Reinstate build-windows

* Fix name

---------

Co-authored-by: Derek Bailey <derekbailey@google.com>
candysonya pushed a commit to candysonya/flatbuffers that referenced this pull request Jan 21, 2024
* Target .NET Standard 2.1, .NET 6, .NET 8 only

* Remove mono usage

* Fix bat name ref

* Up deps

* Up deps

* Reinstate build-windows

* Fix name

---------

Co-authored-by: Derek Bailey <derekbailey@google.com>
@ccorsano
Copy link

Hi, sorry to jump back on a closed topic !

Just adding a point that might have been missed in this initial conversation: dropping .netstandard2.0 removed the possibility to use Google.FlatBuffers and by extension Flatbuffers generated code when writing Source Generators, which is a very current use case in the .net ecosystem, especially in the context of writing AOT friendly code in .net 6 / .net 8 code.

I can see the strong incentive to consolidate around modern .net, but was there a compelling reason to drop .netstandard2.0 as well ?

@bjornharrtell
Copy link
Collaborator Author

I had no idea about this @ccorsano can you point me to where I can find out more? I don't think I have anything against adding back .net standard 2.0 but it surprises me that it would be a requirement.

@ccorsano
Copy link

Sure !
You can find more details about Source Generators here:
https://learn.microsoft.com/en-us/dotnet/csharp/roslyn-sdk/source-generators-overview

The netstandard2.0 requirement is mentioned in several highlights, and is definitely one of the pain point of an otherwise very nice feature.

I haven't seen any sign that it will be upgraded to netstandard2.1: it stayed netstandard2.0 despite source generators going through a couple major versions and seeing a growing usage in the community.

@bjornharrtell
Copy link
Collaborator Author

I see... however a bit confused about this comment dotnet/roslyn#47087 (comment).

Anyhow, I think we should try to add back netstandard2.0 target. I wonder if the NETSTANDARD2_1 macro is required though.

@ccorsano
Copy link

I see... however a bit confused about this comment dotnet/roslyn#47087 (comment).

Yes, I found that one and got confused as well ... I guess the reason for the netstandard2.0 constraint has to do with IDE support, and particularly Visual Studio.

Anyhow, I think we should try to add back netstandard2.0 target. I wonder if the NETSTANDARD2_1 macro is required though.

Sounds great !
I only skimmed through the changes, so I am not sure what the initial intention was with those #ifdef.
NETSTANDARD2.1 is only defined when targeting that framework, so I find the old code oddly specific.
With the upgrade to .net6 / .net8 reversing the condition with a NETSTANDARD2_0 could be a better option ?

@bjornharrtell
Copy link
Collaborator Author

@ccorsano I suggest we try to simply do #8295 first.

jochenparm pushed a commit to jochenparm/flatbuffers that referenced this pull request Oct 29, 2024
* Target .NET Standard 2.1, .NET 6, .NET 8 only

* Remove mono usage

* Fix bat name ref

* Up deps

* Up deps

* Reinstate build-windows

* Fix name

---------

Co-authored-by: Derek Bailey <derekbailey@google.com>
jochenparm pushed a commit to jochenparm/flatbuffers that referenced this pull request Oct 29, 2024
* Target .NET Standard 2.1, .NET 6, .NET 8 only

* Remove mono usage

* Fix bat name ref

* Up deps

* Up deps

* Reinstate build-windows

* Fix name

---------

Co-authored-by: Derek Bailey <derekbailey@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c# CI Continuous Integration documentation Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants