-
Notifications
You must be signed in to change notification settings - Fork 1
Bump GraphQL version and ASP.Net Core version for .NET Framework #91
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
|
Warning Rate limit exceeded@Shane32 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 14 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR updates project version numbers to 8.0.0-preview and GraphQL dependency to 8.8.0, splits AspNetCore package references by target framework, refactors middleware to use a public property pathway for WebSocket sub-protocols instead of constructor-injected IHostApplicationLifetime, and suppresses obsolete-member compiler warnings across affected constructors. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Directory.Build.props(2 hunks)src/GraphQL.AspNetCore3/GraphQL.AspNetCore3.csproj(2 hunks)src/GraphQL.AspNetCore3/GraphQLHttpMiddleware.cs(3 hunks)src/Samples/Net48Sample/Net48Sample.csproj(2 hunks)src/Tests/Middleware/AuthorizationTests.cs(1 hunks)src/Tests/Middleware/FileUploadTests.cs(1 hunks)src/Tests/Middleware/MiscTests.cs(4 hunks)src/Tests/Middleware/WebSocketTests.cs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/Tests/Middleware/FileUploadTests.cs (2)
src/Tests/Middleware/AuthorizationTests.cs (2)
MyMiddleware(146-160)MyMiddleware(149-153)src/Tests/Middleware/MiscTests.cs (5)
MyMiddleware(64-75)MyMiddleware(68-72)MyMiddleware(122-145)MyMiddleware(125-127)MyMiddleware(129-138)
src/Tests/Middleware/MiscTests.cs (2)
src/Tests/Middleware/AuthorizationTests.cs (2)
MyMiddleware(146-160)MyMiddleware(149-153)src/Tests/Middleware/FileUploadTests.cs (2)
MyMiddleware(62-107)MyMiddleware(67-72)
src/Tests/Middleware/AuthorizationTests.cs (2)
src/Tests/Middleware/FileUploadTests.cs (2)
MyMiddleware(62-107)MyMiddleware(67-72)src/Tests/Middleware/MiscTests.cs (5)
MyMiddleware(64-75)MyMiddleware(68-72)MyMiddleware(122-145)MyMiddleware(125-127)MyMiddleware(129-138)
🪛 GitHub Actions: Run code tests
src/Samples/Net48Sample/Net48Sample.csproj
[error] 1-1: System.Security.AccessControl doesn't support netcoreapp2.1. Consider updating your TargetFramework to netcoreapp3.1 or later.
🔇 Additional comments (14)
Directory.Build.props (1)
4-4: Version bump from 7.1.0-preview to 8.0.0-preview looks appropriate.The version bump aligns with the PR objectives to update versions across the project.
src/Tests/Middleware/AuthorizationTests.cs (1)
148-150: Warning suppression for obsolete IHostApplicationLifetime is appropriate.The pragma warning suppression is a standard approach for maintaining compatibility with obsolete APIs while silencing compiler warnings. This is consistent with the suppression pattern used across other test files in the PR.
src/Tests/Middleware/MiscTests.cs (4)
21-23: LGTM: Proper warning suppression for obsolete API usage in tests.The pragma warning suppression is correctly applied around the obsolete IHostApplicationLifetime usage.
51-53: LGTM: Consistent warning suppression pattern.The suppression is properly applied and consistent with the rest of the file.
67-69: LGTM: Constructor warning suppression is appropriate.The pragma blocks correctly suppress the obsolete member warning for the constructor parameter.
124-130: LGTM: Constructor chain warning suppression is properly implemented.The pragma suppression covers both the parameterless constructor and the constructor with IHostApplicationLifetime parameter, which is the correct approach.
src/Tests/Middleware/WebSocketTests.cs (2)
113-115: LGTM: Warning suppression is properly applied.The pragma blocks correctly suppress the obsolete member warning for the TestMiddleware constructor.
166-173: LGTM: Constructor chain warning suppression is correct.The pragma suppression properly covers the public constructor and the private constructor that accepts IHostApplicationLifetime.
src/GraphQL.AspNetCore3/GraphQLHttpMiddleware.cs (3)
38-40: LGTM: Warning suppression for obsolete IHostApplicationLifetime parameter.The pragma suppression is properly applied to the constructor parameter in the generic middleware class. This maintains backward compatibility while acknowledging the obsolete API.
69-71: LGTM: Field declaration warning suppression is appropriate.The pragma blocks correctly suppress the obsolete member warning for the private field that stores the IHostApplicationLifetime reference.
97-99: LGTM: Constructor parameter warning suppression is correct.The pragma suppression properly handles the obsolete IHostApplicationLifetime parameter in the non-generic middleware constructor.
src/Tests/Middleware/FileUploadTests.cs (1)
66-68: LGTM: Constructor warning suppression is properly applied.The pragma blocks correctly suppress the obsolete member warning for the MyMiddleware constructor parameter. This is consistent with the suppression pattern used throughout the test files.
src/Samples/Net48Sample/Net48Sample.csproj (1)
15-19: Microsoft.AspNetCore 2.3.0 package versions exist on NuGet.All five packages referenced are available in version 2.3.0: Microsoft.AspNetCore, Microsoft.AspNetCore.CookiePolicy, Microsoft.AspNetCore.HttpsPolicy, Microsoft.AspNetCore.Mvc, and Microsoft.AspNetCore.StaticFiles. However, the pipeline failure is unrelated to package availability—it stems from System.Security.AccessControl not supporting netcoreapp2.1, which requires resolving the underlying dependency compatibility issue.
src/GraphQL.AspNetCore3/GraphQL.AspNetCore3.csproj (1)
20-32: Microsoft.AspNetCore 2.3.0 exists and is compatible with netstandard2.0—no issue here.The package explicitly targets .NETStandard 2.0, making it suitable for the netstandard2.0 target framework shown in lines 20–24. The split of package references by target framework is well-structured and the versions are correct.
Pull Request Test Coverage Report for Build 19692126247Details
💛 - Coveralls |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.