PetaPoco icon indicating copy to clipboard operation
PetaPoco copied to clipboard

v7

Open asherber opened this issue 5 years ago • 12 comments

Even though v6 hasn't been out for all that long, I think there are enough breaking changes that have been discussed (#478, #500 for example) and side conversations about code cleanup that it might be worth considering a v7 to roll all of these up.

In addition to the two issues above, I think it would be good to:

  1. Reorganize folders/namespaces a little.
  2. Move convenience functions out of classes and into extension methods. For example, there are three overloads of Database.Query(), but only one of them does work; the other two delegate to the third. Similarly, all of the Fetch() methods just delegate to other methods to do their work. Having all of these in the base class clutters the class and clutters the interface, making it harder for anyone to write their own implementation of IDatabase (which, I know, is kind of unlikely, but still). By moving them all out to extension methods on IDatabase, we keep the functionality but reduce the clutter, and any IDatabase gets them for free.

I'm interested in discussion on this, and I'd be happy to take a first pass at the work.

asherber avatar May 17 '19 20:05 asherber

Agree. I’ll add a few more details around items I’d like to address in v7 soon.

pleb avatar May 18 '19 04:05 pleb

So this, below, has been lifted from the roadmap

  • Implement logging infrastructure
  • Composite key support
  • Version support (optimistic concurrency)
  • Remove Homemade Cache class

Which brings me to:

  • Update roadmap with version 7 milestones

And

I'd like to also look at including https://github.com/CollaboratingPlatypus/PetaPoco/pull/149 which would remove the IL gen code.

There are possibly more opportunities to reduce code duplication which was added during working on the async feature.

Lastly, I'd like to also rework the build/ci integration. But, before going all in on this, I still want to see/explore what Github Actions can do, as it might be a good fit for replacing AppVeyor. For now, I will fix the build failing for PRs and will also enable unit tests and integration tests for SQL Server, MySQL and Postgresql.


Ok, now that I've laid that out on the table I realise it's probably a little too much, so I'd like to suggest the following:

V7.0

  • @asherber code refactor
  • @pleb fix CI and include running tests

v7.1

  • Composite key support
  • Version support (optimistic concurrency)
  • Furthur deduplication of code

v7.2

  • Implement logging infrastructure
  • Remove Homemade Cache class

v7.3

  • https://github.com/CollaboratingPlatypus/PetaPoco/pull/149 (Possibly)

pleb avatar May 20 '19 10:05 pleb

Yes, I think the most important thing is to get a handle on all the breaking changes and make sure those go into 7.0. After that, and hopefully with a tighter codebase, improvements are the next step.

For logging, have you seen https://github.com/damianh/LibLog ?

asherber avatar May 20 '19 12:05 asherber

Hey, awesome what you are doing here...is there also a timeline for releasing? All features makes absolutely sense and would be great to use

nhaberl avatar Sep 02 '19 17:09 nhaberl

I'd like to make a proposal. PP should integrate the nullable reference types feature in c# v8

https://docs.microsoft.com/en-us/dotnet/csharp/tutorials/nullable-reference-types https://docs.microsoft.com/en-us/dotnet/csharp/tutorials/upgrade-to-nullable-references

ArgoZhang avatar Nov 06 '19 02:11 ArgoZhang

How do you think they should be used? Also, keep in mind that they’re only available in .NET Core 3.

asherber avatar Nov 06 '19 03:11 asherber

bro. nullable reference types is a c# language feature. You'll need to set up your machine to run .NET Core, including the C# 8.0 compiler. The C# 8.0 compiler is available with Visual Studio 2019, or .NET Core 3.0.

ArgoZhang avatar Nov 06 '19 03:11 ArgoZhang

C#8 is only supported on .NET Core 3, not on .NET Core 2 or any Framework versions.

https://devblogs.microsoft.com/dotnet/building-c-8-0/ https://devblogs.microsoft.com/dotnet/announcing-net-standard-2-1/ https://docs.microsoft.com/en-us/dotnet/standard/net-standard

asherber avatar Nov 06 '19 04:11 asherber

you are right. so as above said this feature is a language feature. it is only working in complier process. it nothing changed when pp lib generated. this is only null reference types check in complie step. it will show more warning about the type maybe null. we only upgrade ide from 2017 to 2019 16.3+. that’s all.

| | Argo | | 邮箱:[email protected] |

Signature is customized by Netease Mail Master

On 11/06/2019 12:17, Aaron Sherber wrote:

C#8 is only supported on .NET Core 3, not on .NET Core 2 or any Framework versions.

https://devblogs.microsoft.com/dotnet/building-c-8-0/ https://devblogs.microsoft.com/dotnet/announcing-net-standard-2-1/ https://docs.microsoft.com/en-us/dotnet/standard/net-standard

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

ArgoZhang avatar Nov 06 '19 04:11 ArgoZhang

I'm sorry, but this is not correct. You can only use C# 8 language features in a project that targets .NET Core or .NET Standard 2.1. If PP continues to target .NET Standard 2.0 and Framework versions, it cannot use any C# 8 features.

This page explains how the compiler in VS 2019 automatically picks a language version based on the target frameworks: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/configure-language-version

The attached project demonstrates this. It uses nullable strings and does not compile in VS 2019 because it targets .NET Standard 2.0. If you change TargetFrameworks so that it only targets .NET Standard 2.1, it will compile just fine.

ClassLibrary1.zip

asherber avatar Nov 06 '19 12:11 asherber

I'm sorry, but this is not correct. You can only use C# 8 language features in a project that targets .NET Core or .NET Standard 2.1. If PP continues to target .NET Standard 2.0 and Framework versions, it cannot use any C# 8 features.

That's not correct at all. There are many examples of projects targeting both net461 and netstandard2.0 that use C# 8, including nullable reference types.

See Scrutor as one example. Newtonsoft.Json is another example. I could probably come up with a handful more if you'd like.

khellang avatar Nov 07 '19 08:11 khellang

@khellang You're right -- I didn't think of manually setting the language version when VS wouldn't let me do it in the UI. And most of the info about C#8 says that it's only for .NET Core 3.0 and .NET Standard 2.1.

Long reference article: https://devblogs.microsoft.com/dotnet/try-out-nullable-reference-types/

asherber avatar Nov 07 '19 12:11 asherber