Skip to content

Conversation

embik
Copy link
Member

@embik embik commented Jun 6, 2025

Summary

This is not necessarily something to merge, more of a RFC. I realized we were only using Engage in the provider startup, and it seemed to me it would be a nice idea to use multicluster.Aware instead of the whole mcmanager.Manager kitchen sink. This way, you can use the provider in more custom scenarios (in specific, @xrstf and I are currently re-thinking the api-syncagent to move it to multicluster-runtime, and we might not even use the mcmanager.Manager directly).

Any thoughts on this?

What Type of PR Is This?

/kind cleanup
/kind api-change

Related Issue(s)

Fixes #

Release Notes

Replace `mcmanager.Manager` with `multicluster.Aware` in `apiexport.(p *Provider).Start` function signature

@embik embik requested a review from sttts June 6, 2025 12:23
@kcp-ci-bot kcp-ci-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. dco-signoff: yes Indicates the PR's author has signed the DCO. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API labels Jun 6, 2025
@embik
Copy link
Member Author

embik commented Jun 6, 2025

/hold

for discussion.

@kcp-ci-bot kcp-ci-bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 6, 2025
Copy link

@xrstf xrstf left a comment

Choose a reason for hiding this comment

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

/approve

@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 7, 2025
@kcp-ci-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 0b84ba0deb74a1be1baa673cd74d0178978d784c

@kcp-ci-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xrstf

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

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 7, 2025
@xrstf
Copy link

xrstf commented Jun 11, 2025

We turned out to use the mcmanager after all, so this PR might not be needed anymore.

@embik
Copy link
Member Author

embik commented Sep 1, 2025

/close

for now. We will probably implement something similar once kubernetes-sigs/multicluster-runtime#62 is merged.

@kcp-ci-bot kcp-ci-bot closed this Sep 1, 2025
@kcp-ci-bot
Copy link
Contributor

@embik: Closed this PR.

In response to this:

/close

for now. We will probably implement something similar once kubernetes-sigs/multicluster-runtime#62 is merged.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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. dco-signoff: yes Indicates the PR's author has signed the DCO. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants