refactor ensure work #5660
refactor ensure work #5660
Conversation
karmada-bot
added
the
kind/cleanup
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
karmada-bot
added
the
size/XL
Signed-off-by: changzhen <changzhen5@huawei.com>
XiShanYongYe-Chang
force-pushed
the
refactor-ensure-work
branch
from
fac6a38
to
8413ed6
Compare
karmada-bot
added
size/L
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #5660 +/- ##
==========================================
- Coverage 35.91% 35.86% -0.05%
==========================================
Files 648 648
Lines 45079 45095 +16
==========================================
- Hits 16189 16174 -15
- Misses 27645 27675 +30
- Partials 1245 1246 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
/cc @RainbowMango |
|
OK. will do. |
| type workSyncer struct { | ||
| client client.Client | ||
| interpreter resourceinterpreter.ResourceInterpreter | ||
| overrider overridemanager.OverrideManager | ||
| } |
There was a problem hiding this comment.
The workSyncer holds some immutable parameters, mainly used to reduce the number of parameters passed to ensureWork(). Is this the idea?
There was a problem hiding this comment.
It's like this.
| } | ||
| } | ||
| // ensureWork ensure Work to be created or updated in the target cluster. | ||
| func (s *workSyncer) ensureWork(ctx context.Context, workload *unstructured.Unstructured, binding metav1.Object, targetCluster workv1alpha2.TargetCluster, config extraConfig) error { |
There was a problem hiding this comment.
The ensureWork is dedicated to syncing a single work, which is split from the original ensureWork.
There was a problem hiding this comment.
Yes, more dependencies on single work generation will be extended in extraConfig in the future.
| switch scope { | ||
| case apiextensionsv1.NamespaceScoped: | ||
| // workSyncer is design for syncing work objects as expected. | ||
| type workSyncer struct { |
There was a problem hiding this comment.
I believe it would be more appropriate to have this workSyncer placed in a separate new Go file.
There was a problem hiding this comment.
I plan to change the common.go file name to worksyncer.go when the review is almost complete. This makes it easier to see which parts have been modified during review.
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
The
ensureWorkfunction has some load, and its complexity will continue to increase during ongoing iterations. Therefore, some refactoring has been done on this function.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: