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
Remove init-only properties from pre-.NET 5 targets #3323
Conversation
@@ -3022,6 +3022,7 @@ namespace GraphQL.Validation | |||
public readonly struct ValidationOptions | |||
{ | |||
public ValidationOptions() { } | |||
public ValidationOptions(GraphQL.Types.ISchema schema, GraphQLParser.AST.GraphQLDocument document, System.Collections.Generic.IEnumerable<GraphQL.Validation.IValidationRule>? rules, System.Collections.Generic.IDictionary<string, object?> userContext, GraphQL.Instrumentation.Metrics metrics, GraphQL.Inputs variables, GraphQL.Inputs extensions, GraphQLParser.AST.GraphQLOperationDefinition operation, System.IServiceProvider? requestServices, System.Security.Claims.ClaimsPrincipal? user, System.Threading.CancellationToken cancellationToken) { } |
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.
This is a read-only struct. To allow C# 8 and prior languages to write to these properties, a constructor was added as a non-breaking change. Changing the struct to a read/write struct is a breaking change and affects other code.
#if NET5_0_OR_GREATER | ||
init; | ||
#else | ||
set; | ||
#endif |
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.
Minimize impact here. Perhaps just converting to set;
would be better, whether it is binary-breaking or not.
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.
Note that this class is not likely to be inherited in 3rd-party libraries, so non-source-breaking is probably acceptable here, even if it is binary-breaking.
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.
Changed it to simply set;
since we wouldn't want a class library compiled against .NET Standard 2.0 to assume that the property was an init;
while the main project is compiled against .NET 6 and assumes the property was a set;
therefor having a broken build without any 3rd party references.
Codecov Report
@@ Coverage Diff @@
## master #3323 +/- ##
==========================================
- Coverage 84.04% 84.01% -0.03%
==========================================
Files 376 376
Lines 16179 16201 +22
Branches 2600 2602 +2
==========================================
+ Hits 13597 13612 +15
- Misses 1964 1971 +7
Partials 618 618
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
reviewed |
…TRAC-6694-upgrade-graphql-dotnet-server-to-7.1 * commit '13da37d7c0649cc6186714a6671272af49f06d85': (1255 commits) Add type integrity check when existing types are found (graphql-dotnet#3332) Switch to Nullability.Source (graphql-dotnet#3314) Add missing authorization extensions from field and connection builders (graphql-dotnet#3324) Prevent using graphtype as model (graphql-dotnet#3316) Remove init-only properties from pre-.NET 5 targets (graphql-dotnet#3323) Update UnionGraphType to support CLR references (graphql-dotnet#3320) Bump BenchmarkDotNet from 0.13.1 to 0.13.2 (graphql-dotnet#3313) Bump Microsoft.NET.Test.Sdk from 17.3.0 to 17.3.1 (graphql-dotnet#3311) Fix InputFieldsAndArgumentsOfCorrectLength validation rule (graphql-dotnet#3307) Bump Shouldly from 4.0.3 to 4.1.0 (graphql-dotnet#3304) Add test (graphql-dotnet#3302) !(a is T) -> a is not T (graphql-dotnet#3300) User not duplicated in context (graphql-dotnet#3298) Bump deps (graphql-dotnet#3295) Restore `Field<TGraphType>()` method (graphql-dotnet#3294) Include v7 migration document link in readme (graphql-dotnet#3290) Bump GraphQL-Parser to 8.1.0 (graphql-dotnet#3289) Migration notes updates (graphql-dotnet#3287) Simplify configuration (graphql-dotnet#3286) Introduce ErrorInfoProviderOptions.ExposeExceptionDetailsMode (graphql-dotnet#3276) ... # Conflicts: # docs/package.json # src/GraphQL/GraphQL.csproj # src/GraphQL/Types/GraphTypesLookup.cs
Fixes #3322