-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Add SCC Describer #7127
Add SCC Describer #7127
Conversation
Looks good, please add some representative tests |
@stevekuznetsov ptal |
fixes #5299 |
api.Kind("PersistentVolumeClaim"): &PersistentVolumeClaimDescriber{c}, | ||
api.Kind("Namespace"): &NamespaceDescriber{c}, | ||
api.Kind("Endpoints"): &EndpointsDescriber{c}, | ||
api.Kind("SecurityContextConstraints"): &SecurityContextConstraintsDescriber{c}, |
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.
Why are endpoint and scc kinds plural?
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.
that's how they've been for a while. There is a special case for them in rest mapping for when the singular == the plural.
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.
/twitch
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.
/twitch
yes. tough.
@pweil- need to update generated bash completions |
1f6d07c
to
290b806
Compare
@stevekuznetsov - updated - PTAL |
Could you post example output for the changes? |
Changes to move type inline
|
actually, I'll me add a len check on the ranges as well. Let me know if there is anything else you see Edit: on second though, knowing they are empty provides value...it indicates that they will pull from the namespace depending on the strategy. Leaving that alone |
Instead of empty, print |
It looks good other than the "empty" representation. It would be nice if things lined up and didn't look so awful, but I can't tell why that's occurring as you're using a TabWriter. |
290b806
to
ba00bf0
Compare
|
ba00bf0
to
236a1a4
Compare
LGTM |
[merge] |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5030/) (Image: devenv-rhel7_3478) |
[Test]ing while waiting on the merge queue |
Evaluated for origin test up to 6a835c5 |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/1362/) |
Evaluated for origin merge up to 6a835c5 |
Merged by openshift-bot
fork PR: openshift/kubernetes#5
Adds a describer for SCC. Example output:
Fixes #7051