search

robertvazan / guerrillantp

Simple NTP (SNTP) client library providing .NET applications with accurate network time.
https://guerrillantp.machinezoo.com/
Apache License 2.0
66 stars 17 forks source link

A few things #8

Closed TonyValenti closed 2 years ago

TonyValenti commented 2 years ago
  1. Your code looks great! Just publish the new version whenever you're able to.
  2. I noticed that you're now targeting .NET 5.0 and seem to have dropped support for .NET 4.8. I don't know if that was intentional or not.
  3. I noticed that you're targeting .NET 5.0, but this should likely be 6.0 since 5.0 is out of support.
  4. When I build, I get Analyzer suggestions recommending usage of AsSpan() to increase performance.
  5. Can you enable Discussions on this repo?
robertvazan commented 2 years ago

Thanks for the feedback. You don't have to create new issue for every discussion. You can continue discussion on closed issues and merged pull requests.

Yes, .NET Standard support was intentionally removed. The new async functionality does not work on it anyway. Old 2.01 version will remain available for download, which will be mentioned on the website. AFAIK there's no reason to develop apps for .NET 4.8 anymore. Targeting .NET 5 instead ofd 6 just increases compatibility with no harm. Apps are free to use .NET 6 or any future version of .NET for deployment.

Thanks for the AsSpan() hint. I am using VSCode on linux and I don't get any such warning. I have fixed it now (I think).

RobThree commented 1 year ago

AFAIK there's no reason to develop apps for .NET 4.8 anymore.

Agreed, NEW applications shouldn't target .Net Framework anymore, however, there's tons of applications that have been developed over the past 20 years that are "stuck" on .Net Framework (for the foreseeable future) because porting them to newer .Net versions is a huge undertaking for some applications and some organisations or single developers may not have the resources to upgrade anytime soon. I just wanted to point this out.