Update/publish nats#289
Conversation
🎨 Remove omitempty from app_type(hpa.enabled)
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey abdheshnayak - Here's my review!
General suggestions:
- Ensure that the renaming and changes in the NATS subject formats are reflected and compatible with all subscribers to maintain system integrity.
- Review the implications of making certain fields non-nullable in the GraphQL schema, particularly for
enabledin HPA configurations, to ensure client compatibility. - Given the typo in the
IsReadyOnlyfield, consider a thorough search for similar issues that might affect clarity and correctness. - Verify the broader impact of these changes on related components and services to ensure seamless integration and functionality.
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
| EnvironmentName func(childComplexity int) int | ||
| ID func(childComplexity int) int | ||
| Immutable func(childComplexity int) int | ||
| IsReadyOnly func(childComplexity int) int |
There was a problem hiding this comment.
issue (llm): It appears there's a typo in the field name IsReadyOnly. Based on the context, it seems like it should be IsReadOnly. This typo could lead to confusion and bugs in the future.
| return string(obj.Id), nil | ||
| } | ||
|
|
||
| // IsReadyOnly is the resolver for the isReadyOnly field. |
There was a problem hiding this comment.
issue (llm): It appears that the IsReadyOnly resolver method throws a not implemented panic. To ensure the new isReadyOnly field works as expected, it's crucial to implement this resolver method and add unit tests verifying its correct behavior, especially considering different states of a Secret object.
|
|
||
| return e.complexity.Secret.Immutable(childComplexity), true | ||
|
|
||
| case "Secret.isReadyOnly": |
There was a problem hiding this comment.
suggestion (llm): I noticed that the isReadyOnly field has been added to the GraphQL schema for Secret. It's important to add integration tests for the GraphQL queries and mutations that involve this new field to ensure it's correctly handled in all relevant operations.
| MinReplicas *int `json:"minReplicas,omitempty"` | ||
| ThresholdCPU *int `json:"thresholdCpu,omitempty"` | ||
| ThresholdMemory *int `json:"thresholdMemory,omitempty"` | ||
| Enabled bool `json:"enabled"` |
There was a problem hiding this comment.
suggestion (llm): The change from a pointer to a non-pointer boolean for the Enabled field in GithubComKloudliteOperatorApisCrdsV1Hpa and GithubComKloudliteOperatorApisCrdsV1HPAIn structs implies that this field is now required and will default to false if not provided. Please ensure there are tests to verify that the GraphQL mutations and inputs behave as expected when this field is omitted, particularly in scenarios where it was previously optional.
| } | ||
|
|
||
| func clusterBuildCacheUpdateSubject(buildCache *entities.BuildCacheKey) string { | ||
| return fmt.Sprintf("res-updates.account.%s.displayName.%s.name.%s", |
There was a problem hiding this comment.
suggestion (llm): The change in the format of the NATS subject for build cache updates removes displayName and name, relying solely on id. It's important to verify through integration tests that subscribers to these subjects (if any exist) correctly handle and process messages with the new subject format. This ensures that the system's overall functionality remains intact.
| return graphql.Null | ||
| } | ||
| if resTmp == nil { | ||
| if !graphql.HasFieldError(ctx, fc) { |
There was a problem hiding this comment.
suggestion (llm): The addition of null checks in the GraphQL execution context for HPA.enabled suggests a stricter handling of null values. Please ensure there are tests covering scenarios where HPA.enabled might be null, especially in light of the schema changes making enabled a non-nullable boolean. This is crucial for maintaining the robustness of the GraphQL API.
2 participants
No description provided.