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

Add .NET Standard 2.0 target #1436

Merged
merged 76 commits into from
Dec 26, 2024
Merged

Add .NET Standard 2.0 target #1436

merged 76 commits into from
Dec 26, 2024

Conversation

AArnott
Copy link
Contributor

@AArnott AArnott commented Dec 20, 2024

As discussed in #1415, here is the change to adds .NET Standard 2.0 targeting.

@AArnott
Copy link
Contributor Author

AArnott commented Dec 20, 2024

@thomhurst I got the 3 projects we discussed building. I haven't done anything with the tests to ensure they test the functionality on .NET Framework, which I expect is important. There is also some odd build break that is related to the source generator.

Can you take it from here?

@thomhurst
Copy link
Owner

Thanks @AArnott ! I'll take a look when I get some time

@thomhurst
Copy link
Owner

@AArnott looks like I've finally got it working, with the caveat of users need to bring in a poly fill library or add a definition for the ModuleInitializerAttribute to allow the source generated code to register itself

@AArnott
Copy link
Contributor Author

AArnott commented Dec 25, 2024

That is awesome. Your source generator could look to see if the module initializer attribute was already defined in the compilation and if not it could define it itself.

@thomhurst
Copy link
Owner

That is awesome. Your source generator could look to see if the module initializer attribute was already defined in the compilation and if not it could define it itself.

I was thinking that but if other polyfill libraries are doing the same it'd cause type conflicts. And source generators can't see other source generated code so I wouldn't be able to counter that scenario. So might just have to be a caveat that users define or polyfill it themselves.

@AArnott
Copy link
Contributor Author

AArnott commented Dec 25, 2024

Good point.

@thomhurst thomhurst marked this pull request as ready for review December 26, 2024 12:11
@thomhurst thomhurst merged commit 7266461 into thomhurst:main Dec 26, 2024
6 of 8 checks passed
@thomhurst
Copy link
Owner

@AArnott this should now be working in 0.6.0

@AArnott
Copy link
Contributor Author

AArnott commented Dec 26, 2024

I'll try it out on Nerdbank.MessagePack right away. Thanks.

@AArnott
Copy link
Contributor Author

AArnott commented Dec 26, 2024

FYI I was able to get my analyzers test project (which is the smaller of my two test projects) running on TUnit. It required several workarounds however.

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.

2 participants