Conversation
Summary of ChangesHello @olavloite, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on initiating the upgrade path for the Google Cloud Entity Framework Core Spanner provider to align with .NET 10 and prepare for Entity Framework Core v10. The primary changes involve updating the target framework across various project configurations and making a small, targeted adjustment in an integration test to maintain functionality with the newer versions. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request begins the process of upgrading to .NET 10 and Entity Framework Core 10. The target framework has been updated across all projects, which is a good first step. However, the Entity Framework Core package references are still pointing to version 8.x. These will need to be updated to a preview version of EF Core 10 for the projects to be fully aligned with the goal of this PR. I've also left a minor suggestion for code style improvement in one of the test files.
Google.Cloud.EntityFrameworkCore.Spanner.IntegrationTests/QueryTests.cs
Outdated
Show resolved
Hide resolved
04eb18e to
1dba885
Compare
Target Entity Framework Core 10 and .NET 10.
d33d450 to
f85e8d7
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a significant but necessary upgrade, targeting .NET 10 and Entity Framework Core 10. The changes are extensive, touching many parts of the codebase from project files to internal provider logic and tests.
Key changes include:
- Bumping TargetFramework and package versions.
- Adapting to API changes in EF Core 10, especially in migrations, query processing, and service registration.
- Updating tests to match the new SQL parameter naming convention from EF Core.
The refactoring of SpannerHistoryRepository is a notable improvement, making it more robust and correctly handling Spanner's limitations regarding INFORMATION_SCHEMA queries within transactions. The introduction of SpannerSqlTranslatingExpressionVisitor for GREATEST and LEAST functions is also a clean implementation aligning with new EF Core extension points.
Overall, the changes are well-implemented and crucial for keeping the provider up-to-date. The only minor opportunities for cleanup identified are the removal of a couple of unused using statements.
Google.Cloud.EntityFrameworkCore.Spanner/Query/Internal/SpannerMathTranslator.cs
Outdated
Show resolved
Hide resolved
Google.Cloud.EntityFrameworkCore.Spanner/Query/Internal/SpannerMethodCallTranslatorProvider.cs
Outdated
Show resolved
Hide resolved
f85e8d7 to
4de105f
Compare
Google.Cloud.EntityFrameworkCore.Spanner/Query/Internal/SpannerContainsExpression.cs
Outdated
Show resolved
Hide resolved
...e.Cloud.EntityFrameworkCore.Spanner/Query/Internal/SpannerSqlTranslatingExpressionVisitor.cs
Outdated
Show resolved
Hide resolved
Google.Cloud.EntityFrameworkCore.Spanner/Google.Cloud.EntityFrameworkCore.Spanner.csproj
Outdated
Show resolved
Hide resolved
Google.Cloud.EntityFrameworkCore.Spanner.Tests/EntityFrameworkMockServerTests.cs
Outdated
Show resolved
Hide resolved
Google.Cloud.EntityFrameworkCore.Spanner.Tests/EntityFrameworkSessionLeakMockServerTests.cs
Outdated
Show resolved
Hide resolved
d957dfe to
9bb6557
Compare
Target Entity Framework Core v10 and .NET 10.