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

feat(framework): target netstandard2.0 and net8.0 #90

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

riezebosch
Copy link

@riezebosch riezebosch commented Jan 14, 2025

I've reintroduced netstandard2.0 support to enable usage from a net48 project. Unfortunately, we're still tied to net48 due to WCF incompatibilities. 😢

@dgm9704
Copy link
Owner

dgm9704 commented Jan 15, 2025

Hi, and thank you for your interest in the project!
I haven't had time yet to fully analyze the situation.
My first reaction is that this way of doing support for older framework is something that I would explicitly like to avoid.
It might simple and low effort now, but my experience tells me that going forward it will probably only get more complicated, to the point it might even hinder possible future work.
I understand that it might not be suitable for you, but have you at least considered using the legacy branch?
Maybe if you could describe your environment, use case, requirements, timescales etc. a bit more it could help me come to some conclusion.
(We can have discussion in private via email if that is needed.)

@riezebosch
Copy link
Author

riezebosch commented Jan 15, 2025

The truth is, compiler directives aren't strictly necessary, as the only real difference between net9.0 and the netstandard versions is that the latter lacks ArgumentNullException.ThrowIfNull. The legacy branch encountered errors from the SBR-NL endpoint during integration, which did not occur with v3.

I made a serious effort to include System.ArgumentNullException as a polyfill (similar to this example), but the challenge is that there's no straightforward way to throw a System.ArgumentNullException in that context, as it would reference its own type.

@riezebosch
Copy link
Author

riezebosch commented Jan 15, 2025

based on this article I removed netstandard2.1 since it doesn't seem to add any value, netstandard2.0 is still the way if you want to target both full framework and dotnet core.

@riezebosch riezebosch changed the title feat(framework): target netstandard2.0; netstandard2.1 and net8.0 feat(framework): target netstandard2.0 and net8.0 Jan 15, 2025
@dgm9704
Copy link
Owner

dgm9704 commented Jan 17, 2025

I appreciate that you have a situation where you need to support older framework.
I did make a deliberate choice about the branching to main/legacy, understanding
that it might lead to difficult choices.
The difficult choice and my decision here is that without some very compelling reasons
the main branch will not add backwards compatibility.

If there is a problem in the legacy branch that prevent usage, please report a bug
with the repro steps etc so we can move forward by fixing that.
I mentioned the legacy branch will only get critical bug fixes, but that can be relaxed if needed,
even adding new features. The point there was that I have limited resources and they will
be directed towards the main branch.

Again, thank you for taking the time and making an effort towards this library!
I am hopeful we will find a solution that helps your use case while still adhering to my opinionated handling of the project.

@riezebosch
Copy link
Author

riezebosch commented Jan 17, 2025

What specific features do you expect to use in this library? Modern language features are already accessible through PolySharp, and given the minimal external dependencies, I’m struggling to see what additional framework features would be relevant 🤷🏻.

The main limitation right now is the absence of ArgumentNullException.ThrowIfNull in netstandard2.0.

Update

Maybe you like this implementation better? Less use of compiler directives, only for the method inlining which could also be considered optional: main...riezebosch:Xoxo:feat/netstandard2.0

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.

3 participants