Skip to content
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

✨ *: use cluster-aware keyfuncs & splitters #1958

Conversation

stevekuznetsov
Copy link
Contributor

go.mod: use newest kcp-dev/apimachinery

Signed-off-by: Steve Kuznetsov skuznets@redhat.com


*: use cluster-aware keyfuncs & splitters

Signed-off-by: Steve Kuznetsov skuznets@redhat.com


Part of #209
Generated with kcp-dev/code-generator#53

/assign @ncdc @sttts

@openshift-ci openshift-ci bot assigned ncdc and sttts Sep 12, 2022
@openshift-ci openshift-ci bot requested review from csams and jmprusi September 12, 2022 23:09
@openshift-ci openshift-ci bot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Sep 12, 2022
@stevekuznetsov stevekuznetsov force-pushed the skuznets/use-cluster-aware-keyfunc branch from 31ee1b5 to 5f672d1 Compare September 12, 2022 23:09
if err != nil {
runtime.HandleError(err)
return nil
}
// e.g. root:org<separator>ws
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, crap - Need to do a lot more work to get comments munged correctly ... would likely need to merge this with any from the first statement, since this SplitClusterAwareKey node gets removed entirely

@ncdc
Copy link
Member

ncdc commented Sep 13, 2022

I think there are legit failures from the changes here?

@stevekuznetsov
Copy link
Contributor Author

That's ... concerning. I'll check tomorrow - syncer unit tests likely are from the test fakes?

@stevekuznetsov stevekuznetsov force-pushed the skuznets/use-cluster-aware-keyfunc branch 2 times, most recently from 159d0a2 to c579202 Compare September 14, 2022 16:55
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 14, 2022
@stevekuznetsov stevekuznetsov force-pushed the skuznets/use-cluster-aware-keyfunc branch from c579202 to 230a885 Compare September 14, 2022 16:57
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 14, 2022
@stevekuznetsov stevekuznetsov force-pushed the skuznets/use-cluster-aware-keyfunc branch 2 times, most recently from 8a8f90b to d303897 Compare September 14, 2022 17:53
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
These tests are ripe for being updated to assume less about the system.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
@stevekuznetsov stevekuznetsov force-pushed the skuznets/use-cluster-aware-keyfunc branch from ba95b60 to 25fdd02 Compare September 14, 2022 19:20
@stevekuznetsov
Copy link
Contributor Author

OK, rebased on all re-reqs. Passing locally. @ncdc ready for review

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 15, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 15, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ncdc

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 15, 2022
@openshift-merge-robot openshift-merge-robot merged commit cf0235c into kcp-dev:main Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants