-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Stop using apiserver for generating docs #126
Conversation
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.
With my little knowledge about this subject, have a few questions.
@@ -25,12 +25,12 @@ steps: | |||
args: [fetch, --depth=100] | |||
- name: "ubuntu" | |||
args: ["mkdir", "-p", "/workspace/_output/linux_amd64"] | |||
- name: "gcr.io/kubebuilder/thirdparty-linux:1.10" | |||
- name: "gcr.io/kubebuilder/thirdparty-linux:1.10.1" |
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.
I am assuming we already have thirdparty-linux:1.10.1 already in gcr ? otherwise all build triggers will fail.
If not, then one way to address it would be to have thirdparty tools docker-file and cloudtools.yml file committed in a separate check-in and have thirdparty image 1.10.1 ready and then check in the rest.
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.
We already have this. I pushed it :)
cmd/kubebuilder/docs/docs.go
Outdated
|
||
// Create the go program to create the swagger.json by serializing the openapi go struct | ||
cr := util.GetCopyright(copyright) | ||
doSwagger(wd, swaggerTemplateArgs{ |
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.
May be rename it to doSwaggerGenerator ? because it is generating a generator Go program not the swagger itself ?
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.
Done
cmd/kubebuilder/docs/docs.go
Outdated
util.Repo, | ||
}) | ||
// Run the go program to write the swagger.json output to a file | ||
c := exec.Command("go", "run", filepath.Join("pkg", "openapi", "cmd", "main.go")) |
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.
It isn't very clear to me why it has to be a separate program in that project to do this ? Why this functionality can't be embedded in KB itself ?
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.
This also means "GoPkg.toml" needs to capture these dependencies for this command ?
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.
This would require an update to openapi-gen
, which is possible, but would take some time.
@@ -85,7 +85,7 @@ ENV GOOS darwin | |||
ENV GOARCH amd64 | |||
ENV DEST /usr/local/kubebuilder/bin/ | |||
RUN mkdir -p $DEST || echo "" | |||
RUN git clone https://github.com/kubernetes-incubator/reference-docs $GOPATH/src/github.com/kubernetes-incubator/reference-docs --depth=1 | |||
RUN git clone https://github.com/kubernetes-incubator/reference-docs $GOPATH/src/github.com/kubernetes-incubator/reference-docs --branch kubebuilder --depth=1 |
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.
Are the changes in kubebuilder branch will ever be merged in master for that repo ?
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.
Probably not for a long time if ever. The branch moves away from using the endpoints to find resources, and instead uses the paths. Reference-docs hasn't had much active development AFAIK.
|
||
func AddDocs(cmd *cobra.Command) { | ||
docsCmd.Flags().BoolVar(&cleanup, "cleanup", true, "If true, cleanup intermediary files") | ||
docsCmd.Flags().BoolVar(&verbose, "verbose", true, "If true, use verbose output") | ||
docsCmd.Flags().BoolVar(&generateConfig, "generate-config", true, "If true, generate the docs/reference/config.yaml.") | ||
docsCmd.Flags().StringVar(&outputDir, "output-dir", filepath.Join("docs", "reference"), "Build docs into this directory") | ||
docsCmd.Flags().StringVar(©right, "copyright", filepath.Join("hack", "boilerplate.go.txt"), "Location of copyright boilerplate file.") | ||
docsCmd.Flags().StringVar(&generatecmd.Docscopyright, "docs-copyright", "<a href=\"https://github.com/kubernetes/kubernetes\">Copyright 2018 The Kubernetes Authors.</a>", "html for the copyright text on the docs") |
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.
What's the reason of using two different copyright here? Can we set Docscopyright
the same as copyright
?
cmd/kubebuilder/generate/generate.go
Outdated
@@ -164,7 +168,7 @@ func RunGenerate(cmd *cobra.Command, args []string) { | |||
} | |||
} | |||
|
|||
if generators.Has("openapi-gen") { | |||
if doGen("openapi-gen") { |
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.
Does this mean "openapi-gen" will be called by default?
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.
LGTM
Move away from using apiserver-builder for generating docs