-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
🌱 Deprecate DefaultIndex usage and remove where not needed #8855
🌱 Deprecate DefaultIndex usage and remove where not needed #8855
Conversation
// | ||
// Deprecated: This variable is deprecated and will be removed in a future release of Cluster API. | ||
// Instead please use `[]Index{NodeProviderIDIndex}`. | ||
var DefaultIndexes = []Index{NodeProviderIDIndex} |
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.
Decided to deprecate this as I think that the naming is possibly one reason it showed up so many places - possibly confused with index.AddDefaultIndexes
. As there's only one index here I think it's fine to just explicitly define the array.
/area clustercachetracker |
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.
nice cleanup!
one small nit from my side
2c6401f
to
22b6026
Compare
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.
one nit
Signed-off-by: killianmuldoon <kmuldoon@vmware.com>
22b6026
to
2f8bc8d
Compare
Thank you. Nice fix! :) /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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 |
LGTM label has been added. Git tree hash: 9d1b343df809de62b838c4fc19c6805e7bebdaf6
|
Deprecate the
remote.DefaultIndex
due to confusing naming and replace its current valid usages with[]Index{NodeProviderIDIndex}
instead.Remove the use of the index entirely from places where it is not currently used: