docs: Add Cobra docs generator and CI to ensure they remain up to date #2336
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
froblesmartin
changed the title
froblesmartin
force-pushed
the
docs/cobra-autogen
branch
4 times, most recently
from
9d9f17a to
f2b55eb
Compare
froblesmartin
force-pushed
the
docs/cobra-autogen
branch
from
f2b55eb to
2c40222
Compare
froblesmartin
force-pushed
the
docs/cobra-autogen
branch
from
2c40222 to
b20fd7e
Compare
|
Thanks for this @froblesmartin! 👏 I will try and take a pass at reviewing the PR sometime this week 😄 |
This is similar to GoogleCloudPlatform/cloud-sql-proxy#2336, except for AlloyDB AuthProxy.
| os.Exit(1) | ||
| } | ||
|
|
||
| outDir, err := genutils.OutDir(path) |
There was a problem hiding this comment.
I have created a similar change as this PR for AlloyDB: GoogleCloudPlatform/alloydb-auth-proxy#730.
I was getting govulncheck error when using the genutils library here: https://github.com/GoogleCloudPlatform/alloydb-auth-proxy/actions/runs/12130089556/job/33819727861.
I think we can change this to outDir, err := filepath.Abs(path) instead.
There was a problem hiding this comment.
Sounds good, I saw it has some extra checks, but I think it is not worth it. One less dependency 😄
| path := "docs/cmd" | ||
| if len(os.Args) == 2 { | ||
| path = os.Args[1] | ||
| } else if len(os.Args) > 2 { |
There was a problem hiding this comment.
It's cleaner to check for valid input first and then proceed with usual operations.
if len(os.Args) > 2 {
// ... error case
os.Exit(1)
}
path := "docs/cmd"
if len(os.Args) == 2 {
// ... etc
}There was a problem hiding this comment.
Sounds good 😉 Applied
This is similar to GoogleCloudPlatform/cloud-sql-proxy#2336, except for AlloyDB AuthProxy.
+1 @froblesmartin let me know if you need me to take over the two nits otherwise LGTM as well! |
|
@enocom @jackwotherspoon @rhatgadkar-goog changes applied 😉 I have also added the same text and link in the README as @rhatgadkar-goog did in the AlloyDB PR to keep it standardised. EDIT: I have just realised about this 5829a5d. This means, any PR after the last day the docs were updated, will fail, as the new workflow will fail due to the date 🤔 I am going to try to see if that can be avoided from the generator, or if not, omit that in the CI. EDIT2: Solved with the last commit d26f22e. I have also opened a PR to fix this in the AllowDB repo GoogleCloudPlatform/alloydb-auth-proxy#732 |
You are welcome! 😃 Thanks for the quick attention! |
As discussed at #2263, I think this could be integrated into the project.
If you agree with it, I can also help with #2064 by cleaning up duplicates in this PR or a new one, and referencing the new docs in the repo's main README.
Based in https://github.com/kubernetes/kubernetes/blob/master/cmd/gendocs/gen_kubectl_docs.go