Api doc update #297
Api doc update #297
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
| [pkg/apis/authentication/v1beta1/register.go](https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/authentication/v1beta1/register.go); | ||
| The register files must have a var called SchemeBuilder for the | ||
| generated code to reference. There must be an AddToScheme method for | ||
| the installer to reference. |
There was a problem hiding this comment.
Optional: suggest you add something like, "You can look at a group under pkg/apis/... for example register.go files to use as a template, but do not copy the register.go files under pkg/api/...--they are not general."
There was a problem hiding this comment.
agree based on my experiences. Do we have a blurb somewhere about the history of the api group definitions? It is my understanding that api/ is the old stuff and apis/ is the new stuff, but I don't have more specific information or understanding.
|
consider this an lgtm from me with or without my suggested change |
MHBauer
force-pushed
the
api-doc-update
branch
from
66b9b89 to
564a40c
Compare
| [pkg/apis/authentication/v1beta1/register.go](../../pkg/apis/authentication/v1beta1/register.go); | ||
|
|
||
| 3. Add a pkg/apis/`<group>`/install/install.go, which is responsible for adding | ||
| the group to the `latest` package, so that other packages can access the group's |
There was a problem hiding this comment.
Having written an api group recently, I have no idea what this latest and latest.Group text is talking about.
There was a problem hiding this comment.
It turned into the registered package.
MHBauer
force-pushed
the
api-doc-update
branch
from
564a40c to
87821cf
Compare
|
@lavalamp your text included. Additional text and changes in further sections. |
| modified by a controller, the status field can be left off and the fields inside | ||
| the `Spec` can be inlined directly into the struct. | ||
|
|
||
| For each type there should also be a `List` struct. The `List` struct should |
There was a problem hiding this comment.
Nit: I would qualify with "top-level type".
There was a problem hiding this comment.
@lavalamp agree. done.
|
lgtm if you fix that nit. |
MHBauer
force-pushed
the
api-doc-update
branch
from
87821cf to
19daa8a
Compare
lavalamp
added
the
lgtm
|
Thanks a bunch! |
|
@lavalamp Thanks for feedback. I'll be around soon with an apiserver specific change. |
If you are joining the org, please fill out the template below: - [x] I have reviewed the community membership guidelines - [x] I have reviewed the contribution guidelines - [x] I have enabled 2FA on my GitHub account - [x] I have joined the Istio discussion board and subscribed to the Contributors category - [x] I have joined Istio's Slack workspace (fill-out this form to join) List your company name, or indicate Individual if you're not affiliated with a company: IBM Provide a link to at least one PR that you have successfully pushed to one of the Istio repos: istio/istio.io#22051 (Merged)
This is some small updates that are complete as it is. I can turn it into a long running WIP, but I'd rather we focus on small incremental additions for now.
@pmorie @deads2k @lavalamp Is
apis/authenticationstill a good base to template off of for newcomers?