Skip to content

Use NamespacedName Type for Internal APIs #157

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

Closed
danehans opened this issue Jan 6, 2025 · 4 comments · Fixed by #165 or #172
Closed

Use NamespacedName Type for Internal APIs #157

danehans opened this issue Jan 6, 2025 · 4 comments · Fixed by #165 or #172

Comments

@danehans
Copy link
Contributor

danehans commented Jan 6, 2025

Internal APIs, e.g. EndpointSliceReconciler, use separate ns and name strings to represent the ns/name of CRs. A few benefits of using NamespacedName are:

  • Encapsulates the namespace and name into a single struct, improving readability by clearly indicating that these two fields together uniquely identify a Kubernetes resource.
  • Provides stronger type safety compared to using two separate strings.
  • Provides built-in methods like String() that automatically format the resource identifier in the conventional namespace/name format, reducing boilerplate code and potential formatting errors.
  • Functions that operate on Kubernetes resources often require both namespace and name. Using a single types.NamespacedName reduces the number of function parameters, simplifying function signatures.
@ahg-g
Copy link
Contributor

ahg-g commented Jan 7, 2025

and avoids switching the two parameters, which will likely be placed after each other in func parameters

@danehans
Copy link
Contributor Author

danehans commented Jan 8, 2025

The inferncepool and inferencemodel reconcilers are using separate string fields instead of NamespacedName even with #165.

/reopen

@k8s-ci-robot
Copy link
Contributor

@danehans: Reopened this issue.

In response to this:

The inferncepool and inferencemodel reconcilers are using separate string fields instead of NamespacedName even with #165.

/reopen

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-sigs/prow repository.

@MadhavJivrajani
Copy link
Contributor

Apologies @danehans, I've created #172 to fix this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants