-
Notifications
You must be signed in to change notification settings - Fork 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
feat(resource-labels): Add labels flag #637
Conversation
9b3ce13
to
d457ab9
Compare
Hey @dark0dave I appreciate any insights on this change 🙏 |
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
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. |
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
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=true
will 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