-
Notifications
You must be signed in to change notification settings - Fork 309
migrate to record #1665
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
base: master
Are you sure you want to change the base?
migrate to record #1665
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tg123 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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.
Pull Request Overview
This PR migrates the Kubernetes client library to use modern C# features, specifically C# 9 records and C# 11 required fields. The migration replaces traditional classes with records, eliminates constructor-based validation in favor of required field annotations, and renames the IntstrIntOrString type to the more concise IntOrString.
- Converts classes to records for immutable data modeling using C# 9 syntax
- Replaces validation methods with required field annotations from C# 11
- Renames IntstrIntOrString to IntOrString for better naming consistency
Reviewed Changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/KubernetesClient.Tests/KubernetesYamlTests.cs | Updates test class to record and object initializer syntax |
tests/KubernetesClient.Tests/IntOrStringTests.cs | Updates type references from IntstrIntOrString to IntOrString |
tests/E2E.Tests/MinikubeTests.cs | Converts constructor calls to object initializer syntax |
tests/E2E.Aot.Tests/MinikubeTests.cs | Converts constructor calls to object initializer syntax |
src/LibKubernetesGenerator/templates/ModelExtensions.cs.template | Removes entire template file for model extensions |
src/LibKubernetesGenerator/templates/Model.cs.template | Major refactor to generate records with required fields instead of classes with constructors |
src/LibKubernetesGenerator/VersionConverterStubGenerator.cs | Removes entire version converter stub generator |
src/LibKubernetesGenerator/ModelGenerator.cs | Updates to support record generation and type overrides |
src/LibKubernetesGenerator/ModelExtGenerator.cs | Removes entire model extension generator |
src/LibKubernetesGenerator/KubernetesClientSourceGenerator.cs | Removes references to deleted generators |
src/LibKubernetesGenerator/GeneralNameHelper.cs | Removes IValidate interface and updates default parameter value |
src/LibKubernetesGenerator/ClassNameHelper.cs | Adds special handling for int-or-string format |
src/KubernetesClient/Models/*.cs | Updates model classes to records and removes validation methods |
src/KubernetesClient/IValidate.cs | Removes entire validation interface |
examples/*.cs | Updates example code to use object initializers and record syntax |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…tOrString, ResourceQuantity, and V1Patch
…ucts, update implicit conversions, and simplify null checks
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.
Pull Request Overview
Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/KubernetesClient/Models/V1Patch.cs:54
- The constructor validation logic checks if
Content
is null after assignment, but theContent
property has[JsonInclude]
and is settable. Consider making theContent
propertyrequired
instead of performing null validation in the constructor to align with the C# 11 migration described in the PR.
public V1Patch(object body, PatchType type)
{
Content = body;
Type = type;
if (Content == null)
{
throw new ArgumentNullException(nameof(Content), "object must be set");
}
if (Type == PatchType.Unknown)
{
throw new ArgumentException("patch type must be set", nameof(Type));
}
}
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Pull Request Overview
Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Pull Request Overview
Copilot reviewed 33 out of 33 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
This reverts commit 62b20a6.
1 using
record
C# 92 add
required
to field to replace validate() C# 113 IntStrIntOrString -> IntOrString
4 IntOrString and ResourceQuantity are now struct instead of class for performance