Skip to content

Conversation

@manusa
Copy link
Member

@manusa manusa commented Nov 5, 2025

Will facilitate #385

The removed annotations have default values and are not needed. I personally prefer to keep them because it makes it easier for reviewers to understand the tool behaviors.
However, this is problematic with go-sdk since these values aren't marshalled/serialized (omitempty).

The removed annotations have default values and are not needed.
I personally prefer to keep them because it makes it easier for
reviewers to understand the tool behaviors.
However, this is problematic with go-sdk since these values aren't
marshalled/serialized (omitempty).

Signed-off-by: Marc Nuri <marc@marcnuri.com>
@manusa manusa added this to the 0.1.0 milestone Nov 5, 2025
@manusa manusa requested review from Cali0707 and matzew November 5, 2025 11:00
ReadOnlyHint: ptr.To(false),
DestructiveHint: ptr.To(false),
IdempotentHint: ptr.To(false), // TODO: consider replacing implementation with equivalent to: helm upgrade --install
IdempotentHint: nil, // TODO: consider replacing implementation with equivalent to: helm upgrade --install
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why this is nil here and just removed elsewhere?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To preserve the TODO note

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah makes sense

Copy link
Collaborator

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Cali0707 Cali0707 merged commit db3f51d into containers:main Nov 5, 2025
6 checks passed
@manusa manusa deleted the refactor/toolsets branch November 5, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants