Skip to content

[NR-250014] chore: unify reflectors#638

Merged
sigilioso merged 1 commit intomainfrom
NR-250014-unify-reflectors
May 15, 2024
Merged

[NR-250014] chore: unify reflectors#638
sigilioso merged 1 commit intomainfrom
NR-250014-unify-reflectors

Conversation

@sigilioso
Copy link
Copy Markdown
Contributor

This PR removes duplicated code from k8s reflectors module:

Summary of changes

  • DynamicObjectReflector and K8sControllerReflector<K> types are unified into Reflector<K>
  • The namespace parameter is removed from the reflector builder (everywhere else we rely on the default namespace configured when building the client).

Out of scope

  • Simplifying the k8s client: in order to make the review easier, dividing the k8s client responsibilities will be addressed in follow-up PRs.

@sigilioso sigilioso requested review from a team May 14, 2024 14:23
Comment thread super-agent/src/k8s/reflectors.rs
@sigilioso sigilioso force-pushed the NR-250014-unify-reflectors branch from 54dfd3d to 371ce6b Compare May 14, 2024 15:02
Copy link
Copy Markdown
Contributor

@marcsanmi marcsanmi left a comment

Choose a reason for hiding this comment

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

Thanks for addressing it 👍

Comment thread super-agent/src/k8s/reflectors.rs Outdated
Copy link
Copy Markdown
Member

@paologallinaharbur paologallinaharbur left a comment

Choose a reason for hiding this comment

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

🚀

@sigilioso sigilioso force-pushed the NR-250014-unify-reflectors branch from 371ce6b to e45d19f Compare May 15, 2024 06:02
@sigilioso sigilioso merged commit 3b44ea3 into main May 15, 2024
@sigilioso sigilioso deleted the NR-250014-unify-reflectors branch May 15, 2024 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants