Skip to content
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

Genericize kubectl get operations #196

Merged

Conversation

jinnovation
Copy link
Collaborator

Relates to #69.

Currently, we define one function for getting all entities of a given resource type, for each of a finite set of resources, e.g. k8s-kubectl-get-{pods, nodes, ...}. All of these functions are identical save for the resource type that they retrieve.

As such, we generalize to a single implementation—k8s-kubectl-get—that accepts the resource type name and performs the get accordingly.

By extension, we rewire all call sites that assume the existence of per-type get functions to use the genericized counterpart—most notably the kubernetes-state-define-refreshers macro, which currently literally expects functions of those naming pattern to exist.

Similarly, we consolidate unit tests and, finally, remove the granular, per-type get functions.

@codecov-commenter
Copy link

codecov-commenter commented Aug 25, 2021

Codecov Report

Merging #196 (7e3092d) into master (4290924) will decrease coverage by 0.66%.
The diff coverage is 21.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #196      +/-   ##
==========================================
- Coverage   44.27%   43.60%   -0.67%     
==========================================
  Files          28       28              
  Lines        1554     1509      -45     
==========================================
- Hits          688      658      -30     
+ Misses        866      851      -15     
Impacted Files Coverage Δ
kubernetes-commands.el 4.41% <0.00%> (ø)
kubernetes-configmaps.el 0.00% <0.00%> (ø)
kubernetes-deployments.el 0.00% <0.00%> (ø)
kubernetes-ingress.el 0.00% <0.00%> (ø)
kubernetes-jobs.el 19.35% <0.00%> (ø)
kubernetes-namespaces.el 0.00% <0.00%> (ø)
kubernetes-nodes.el 0.00% <0.00%> (ø)
kubernetes-pods.el 10.71% <0.00%> (ø)
kubernetes-secrets.el 0.00% <0.00%> (ø)
kubernetes-services.el 0.00% <0.00%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4290924...7e3092d. Read the comment docs.

Copy link
Collaborator

@noorul noorul left a comment

Choose a reason for hiding this comment

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

LGTM, nice 💯

@jinnovation jinnovation merged commit 3e6870d into kubernetes-el:master Aug 25, 2021
@jinnovation jinnovation deleted the jjin/kubectl-get-generalize branch August 25, 2021 07:12
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.

None yet

3 participants