Skip to content

Conversation

kerthcet
Copy link
Member

@kerthcet kerthcet commented Jun 1, 2025

What this PR does / why we need it

Which issue(s) this PR fixes

part of #350

Special notes for your reviewer

Does this PR introduce a user-facing change?

Add global configmap for default settings.

@InftyAI-Agent InftyAI-Agent added needs-triage Indicates an issue or PR lacks a label and requires one. needs-priority Indicates a PR lacks a label and requires one. do-not-merge/needs-kind Indicates a PR lacks a label and requires one. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 1, 2025
@kerthcet kerthcet force-pushed the cleanup/fix-config branch from 5ba3796 to 99d9f57 Compare June 1, 2025 13:45

logger.V(10).Info("reconcile Service", "Service", klog.KObj(service))

cm := &corev1.ConfigMap{}
Copy link
Member

Choose a reason for hiding this comment

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

I have a question here 🤔 : Do we need to get cm obj every time we reconcile? Or can we use event triggering? Because I understand that cm will not be modified in most cases, it seems meaningless to get it every time in Reconcile. But this is not a blocking item but more of an optimization item. If it is due to the code width problem, I can submit a PR to optimize this part in the near future.

Copy link
Member Author

@kerthcet kerthcet Jun 1, 2025

Choose a reason for hiding this comment

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

Anyway, it's not a big problem. We can optimize this later.

@kerthcet kerthcet force-pushed the cleanup/fix-config branch from 42d466c to 5cacbdf Compare June 2, 2025 02:16
@kerthcet
Copy link
Member Author

kerthcet commented Jun 2, 2025

/lgtm
/kind feature

@InftyAI-Agent InftyAI-Agent added lgtm Looks good to me, indicates that a PR is ready to be merged. feature Categorizes issue or PR as related to a new feature. and removed do-not-merge/needs-kind Indicates a PR lacks a label and requires one. labels Jun 2, 2025
@InftyAI-Agent InftyAI-Agent removed the lgtm Looks good to me, indicates that a PR is ready to be merged. label Jun 2, 2025
kerthcet added 2 commits June 2, 2025 17:27
Signed-off-by: kerthcet <[email protected]>
Signed-off-by: kerthcet <[email protected]>
@kerthcet kerthcet force-pushed the cleanup/fix-config branch from 573646b to 0777ce2 Compare June 2, 2025 09:31
Signed-off-by: kerthcet <[email protected]>
@kerthcet
Copy link
Member Author

kerthcet commented Jun 2, 2025

/lgtm

@InftyAI-Agent InftyAI-Agent added the lgtm Looks good to me, indicates that a PR is ready to be merged. label Jun 2, 2025
@InftyAI-Agent InftyAI-Agent merged commit 24913b1 into InftyAI:main Jun 2, 2025
15 checks passed
@kerthcet kerthcet deleted the cleanup/fix-config branch June 2, 2025 10:50
@kerthcet
Copy link
Member Author

kerthcet commented Jun 3, 2025

cc @carlory

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. feature Categorizes issue or PR as related to a new feature. lgtm Looks good to me, indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a label and requires one. needs-triage Indicates an issue or PR lacks a label and requires one.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants