.NETFramework vs .NET(Core,v5 or newer)

alexalok / dotAPNS

dotAPNS is a library used to send push notifications to Apple devices using Apple Push Notification service via HTTP/2 API.

Apache License 2.0

112 stars 34 forks source link

.NETFramework vs .NET(Core,v5 or newer) #105

Closed knocte closed 1 year ago

knocte commented 2 years ago

Hey, the README is a bit confusing when it comes to .NETFramework caveats and whatnot.

I'm wondering, why not just remove .NETFramework support and cleanup the README?

Thanks

alexalok commented 2 years ago

Hi!

While the readme could be confusing at first there are still folks out there who use this library on .NET Framework projects. Quite frankly, even I do.

knocte commented 2 years ago

But .NETFramework projects can consume .NETStandard libraries anyway; and this is a library.

alexalok commented 2 years ago

What you say is true but .NET Standard is, as a name implies, just a standardized set of methods, not a framework. Underlying implementation of this set is done by the actual framework, so running the .NET Standard-targeted version of library on a .NET Framework will still result in the same support problems.

knocte commented 2 years ago

I see, ok.

Well, this is just my 2cents, if I were you, at least, I would:

Try to detect all the unsupported scenarios in .NETFramework and throw a NotSupportedException().
Clean the README file to have only documentation about .NET6 usage (at the end of the day, v6 is already an LTS; mentions of 2.0, 2.1, 2.2 and 3.0 versions make this lib look like very outdated or deprecated); and then have a final note, at the very end, about gotchas when using it with .NETFramework or old .NETCore versions.

Keep up the good work, I plan to test this lib soon.

alexalok commented 2 years ago

Thanks for the feedback!

Readme definitely needs some renovations.

As for exceptions, I believe they're already being thrown for unsupported scenarios.

knocte commented 2 years ago

Another bit I'd like to highlight from the README:

Important: on .NET Core 2.0, 2.1 and 2.2...

This leaves the reader wondering: also newer than 2.2? but if it's older than 2.0 there's no need to initialize?

alexalok commented 2 years ago

Yup, it's really only those 3 versions that need to set a switch. Older versions aren't targeted by the library and newer once have this switch set by default.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.