Skip to content

Conversation

@nmvk
Copy link
Contributor

@nmvk nmvk commented May 20, 2021

New condition ACK.Advisory to set the condition
when any immutable field is modified.

This is needed for aws-controllers-k8s/code-generator#70

@ack-bot ack-bot requested review from a-hilaly and vijtrip2 May 20, 2021 19:11
@nmvk
Copy link
Contributor Author

nmvk commented May 20, 2021

/assign @vijtrip2

// ConditionTypeWarning indicates any warning that may be present in the resource.
// Examples include
// - Modifying an immutable field after it was created
ConditionTypeWarning ConditionType = "ACK.Warning"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I actually would prefer to call this ConditionTypeInfo or ConditionTypeAdvisory instead of ConditionTypeWarning. The word "warning" is generally reserved for serious things that should be addressed ASAP by an operator. In the case of an immutable field being modified in the Spec of a resource, that's definitely not something that needs to be addressed ASAP by an operator (since it's not dangerous or anything)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed this to ConditionTypeAdvisory as someone may ignore ConditionTypeInfo without noticing

New condition ACK.Warning to set the condition
when any immutable field is modified.
@nmvk nmvk requested a review from jaypipes May 27, 2021 22:00
@nmvk nmvk changed the title Add new condition for warning Add new condition ACK.Advisory May 27, 2021
Copy link
Collaborator

@jaypipes jaypipes left a comment

Choose a reason for hiding this comment

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

👍

@jaypipes
Copy link
Collaborator

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label May 31, 2021
@ack-bot
Copy link
Collaborator

ack-bot commented May 31, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jaypipes, nmvk, RedbackThomson

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:
  • OWNERS [RedbackThomson,jaypipes]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-bot ack-bot merged commit 8191f60 into aws-controllers-k8s:main May 31, 2021
RedbackThomson pushed a commit to RedbackThomson/ack-runtime that referenced this pull request Jun 21, 2021
New condition ACK.Advisory to set the condition
when any immutable field is modified.

This is needed for aws-controllers-k8s/code-generator#70
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants