-
Notifications
You must be signed in to change notification settings - Fork 166
feat(resource-labels): Add labels flag #637
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
Conversation
9b3ce13 to
d457ab9
Compare
|
Hey @dark0dave I appreciate any insights on this change 🙏 |
eyalzek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments as I'm not familiar enough with the project nor the style guide :)
b4670ae to
e7e6812
Compare
migueldelucasdoit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @femrtnz can we have a look at the duplicated code we introduced in this PR?
https://github.com/doitintl/kube-no-trouble/pull/637/checks?check_run_id=30865045590
We can refactor the code to extract an internal function with the duplicated code.
https://www.jetbrains.com/help/go/extract-method.html
52c2b5e to
7f44d35
Compare
|
This PR is too long, style is incorrect and introduces way too many commits. Rebase and clean this up. |
dark0dave
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean this up. Or split into seperate PRS.
|
Printer should not depend on config. Please don't create this binding.... |
Could you please detail what you mean? which file, line etc? it will be easier if you comment on the file. |
|
Also why has the printer changed.... Like this feels brittle. |
5908968 to
f077618
Compare
3167229 to
048a123
Compare
migueldelucasdoit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some typos to fix and some recommendations.
f72aae0 to
c6f59f0
Compare
c6f59f0 to
5f3b830
Compare
|
This pr is massive mistake. Not the functionality the way it was done. |
As part of the issue #405 and and #410 , we are adding a new flag called
--labels.This flag is not active as default
Setting the flag
--labels=truewill include in the output a list of labels for each resource, please check the example output belowExample for TEXT output
Example for CSV output
Example for JSON output
[ { "Name": "test-labels-1", "Namespace": "\u003cundefined\u003e", "Kind": "HorizontalPodAutoscaler", "ApiVersion": "autoscaling/v2beta2", "RuleSet": "Deprecated APIs removed in 1.26", "ReplaceWith": "autoscaling/v2", "Since": "1.23.0", "Labels": { "k8s-app": "gcp-compute-persistent-disk-csi-driver" } } ]