-
-
Notifications
You must be signed in to change notification settings - Fork 0
Implement centralized source code generation and clean up project references #25
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
Conversation
…on and remove DummyExpression class
…meworks in project files
WalkthroughThe PR refactors code generation in the Linqraft source generator by consolidating generated source templates into a dedicated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
examples/Directory.Build.props(0 hunks)src/Linqraft.SourceGenerator/ConstSourceCodes.cs(1 hunks)src/Linqraft.SourceGenerator/SelectExprGenerator.cs(1 hunks)src/Linqraft/DummyExpression.cs(0 hunks)src/Linqraft/Linqraft.csproj(1 hunks)src/Linqraft/packages.lock.json(0 hunks)tests/Directory.Build.props(0 hunks)tests/Linqraft.Tests/Linqraft.Tests.csproj(0 hunks)
💤 Files with no reviewable changes (5)
- examples/Directory.Build.props
- src/Linqraft/DummyExpression.cs
- tests/Linqraft.Tests/Linqraft.Tests.csproj
- tests/Directory.Build.props
- src/Linqraft/packages.lock.json
🧰 Additional context used
🧬 Code graph analysis (1)
src/Linqraft.SourceGenerator/SelectExprGenerator.cs (1)
src/Linqraft.SourceGenerator/ConstSourceCodes.cs (2)
ConstSourceCodes(4-123)ExportAll(7-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (windows-latest)
🔇 Additional comments (4)
src/Linqraft.SourceGenerator/SelectExprGenerator.cs (1)
21-22: LGTM! Clean refactor to centralized source generation.The change to use
ConstSourceCodes.ExportAll(ctx)centralizes the generation of predefined sources (InterceptsLocationAttribute and DummyExpression), improving maintainability and reducing code duplication.src/Linqraft.SourceGenerator/ConstSourceCodes.cs (2)
7-11: LGTM! Clean implementation of centralized source export.The
ExportAllmethod correctly registers both generated sources with appropriate filenames.
14-33: LGTM! InterceptsLocationAttribute template is well-formed.The attribute definition correctly follows the interceptor pattern with appropriate visibility (internal), AttributeUsage configuration, and property initialization.
src/Linqraft/Linqraft.csproj (1)
3-3: The review comment is based on an incorrect assumption about target framework compatibility.Microsoft.CodeAnalysis.CSharp 4.13.0 targets and is compatible with .NETStandard 2.0, and the codebase contains no C# 6.0+ language features or APIs that require newer frameworks.
More importantly, removing specific target frameworks (net6.0, net8.0, net10.0) and targeting netstandard2.0 does not prevent consumers on those frameworks from using the library. Packages targeting netstandard2.0 are forward-compatible and can be consumed by any framework version equal to or newer than .NET Standard 2.0 (including net6.0, net8.0, net10.0). The test and example projects confirm this works correctly—they target net6.0+ and net10.0 while successfully consuming the netstandard2.0-targeted library.
Likely an incorrect or invalid review comment.
Refactor the project to use centralized source code generation through
ConstSourceCodes, removing theDummyExpressionclass and unnecessary project references. This streamlines the codebase and reduces target frameworks in project files.Summary by CodeRabbit
Breaking Changes
Refactor