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

v7 #536

Open
asherber opened this issue May 17, 2019 · 12 comments
Open

v7 #536

asherber opened this issue May 17, 2019 · 12 comments

Comments

@asherber
Copy link
Collaborator

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.

@pleb
Copy link
Member

pleb commented May 18, 2019

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

@pleb
Copy link
Member

pleb commented May 20, 2019

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 #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

v7.1

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

v7.2

  • Implement logging infrastructure
  • Remove Homemade Cache class

v7.3

@asherber
Copy link
Collaborator Author

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 ?

@nhaberl
Copy link

nhaberl commented Sep 2, 2019

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

@ArgoZhang
Copy link
Contributor

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

@asherber
Copy link
Collaborator Author

asherber commented Nov 6, 2019

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

@ArgoZhang
Copy link
Contributor

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.

@asherber
Copy link
Collaborator Author

asherber commented Nov 6, 2019

@ArgoZhang
Copy link
Contributor

ArgoZhang commented Nov 6, 2019 via email

@asherber
Copy link
Collaborator Author

asherber commented Nov 6, 2019

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

@khellang
Copy link

khellang commented Nov 7, 2019

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.

@asherber
Copy link
Collaborator Author

asherber commented Nov 7, 2019

@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/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants