Skip to content
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

Cobradocs sync: improve makefile, resync without commit hash #1904

Draft
wants to merge 24 commits into
base: prod
Choose a base branch
from

Conversation

rohit-nayak-ps
Copy link
Contributor

@rohit-nayak-ps rohit-nayak-ps commented Dec 14, 2024

Description

Updates the Makefile for cobra doc generation related targets:

  • Create a single target for all binaries
  • Add new files automatically to the repo with the Vitess repo hash as part of the commit message
  • Also improved error logging
  • We run make generated-docs as part of this PR to remove commit id from the docs, which was implemented in CobraDocs: Remove commit hash from docs. Fix issue with workdir replacement vitess#17392. Future PRs should only contain modified files since we will no longer every file with commit id on each resync.

Copy link

netlify bot commented Dec 14, 2024

Deploy Preview for vitess ready!

Name Link
🔨 Latest commit 8641d52
🔍 Latest deploy log https://app.netlify.com/sites/vitess/deploys/676dcc250ed740000748a824
😎 Deploy Preview https://deploy-preview-1904--vitess.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps rohit-nayak-ps changed the title Cobradocs sync: improve makefile, resync without commit hash WIP: Cobradocs sync: improve makefile, resync without commit hash Dec 15, 2024
…s empty commit for each binary

Signed-off-by: Rohit Nayak <[email protected]>
@rohit-nayak-ps rohit-nayak-ps marked this pull request as ready for review December 17, 2024 15:22
@rohit-nayak-ps rohit-nayak-ps changed the title WIP: Cobradocs sync: improve makefile, resync without commit hash Do Not Merge: Cobradocs sync: improve makefile, resync without commit hash Dec 17, 2024
@rohit-nayak-ps rohit-nayak-ps marked this pull request as draft December 17, 2024 15:22
@rohit-nayak-ps rohit-nayak-ps marked this pull request as ready for review December 26, 2024 21:37
@rohit-nayak-ps rohit-nayak-ps requested review from deepthi, GuptaManan100 and a team December 26, 2024 21:39
@rohit-nayak-ps rohit-nayak-ps changed the title Do Not Merge: Cobradocs sync: improve makefile, resync without commit hash Cobradocs sync: improve makefile, resync without commit hash Dec 26, 2024
# For a specific version, you can specify COBRADOC_VERSION_PAIRS as an environment variable, Example:
# `make mysqlctl-docs COBRADOC_VERSION_PAIRS="main:22.0" VITESS_DIR=~/go/src/github.com/vitessio/vitess`
%-docs:
go run ./tools/cobradocs/ --vitess-dir "${VITESS_DIR}" --version-pairs "${COBRADOC_VERSION_PAIRS}" $(patsubst %-docs,%,$@)
Copy link
Collaborator

@mattlord mattlord Dec 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't specify anything for COBRADOC_VERSION_PAIRS it does not work:

❯ make generated-docs
go run ./tools/cobradocs/ --vitess-dir "/Users/matt/git/vitess" --version-pairs ""main:22.0,v21.0.0:21.0,v20.0.3:20.0,v19.0.7:19.0"" mysqlctl
2024/12/26 16:58:00 exit status 1
exit status 1
make: *** [mysqlctl-docs] Error 1

❯ git status
On branch rohit/slimmer-docs
Your branch is ahead of 'origin/rohit/slimmer-docs' by 1 commit.
  (use "git push" to publish your local commits)

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	content/en/docs/21.0/reference/programs/mysqlctl/_index.md-e

nothing added to commit but untracked files present (use "git add" to track)

❯ git diff
diff --git a/content/en/docs/21.0/reference/programs/mysqlctl/mysqlctl_init.md b/content/en/docs/21.0/reference/programs/mysqlctl/mysqlctl_init.md
index 7d63ff46..5528f87a 100644
--- a/content/en/docs/21.0/reference/programs/mysqlctl/mysqlctl_init.md
+++ b/content/en/docs/21.0/reference/programs/mysqlctl/mysqlctl_init.md
@@ -44,7 +44,7 @@ mysqlctl \
       --config-file string                                          Full path of the config file (with extension) to use. If set, --config-path, --config-type, and --config-name are ignored.
       --config-file-not-found-handling ConfigFileNotFoundHandling   Behavior when a config file is not found. (Options: error, exit, ignore, warn) (default warn)
       --config-name string                                          Name of the config file (without extension) to search for. (default "vtconfig")
-      --config-path strings                                         Paths to search for config files in. (default [<WORKDIR>])
+      --config-path strings                                         Paths to search for config files in. (default [/Users/matt/git/vitess])
       --config-persistence-min-interval duration                    minimum interval between persisting dynamic config changes back to disk (if no change has occurred, nothing is done). (default 1s)
       --config-type string                                          Config file type (omit to infer config type from file extension).
       --db-credentials-file string                                  db credentials file; send SIGHUP to reload this file
...


❯ export COBRADOC_VERSION_PAIRS="main:22.0"

❯ make generated-docs
go run ./tools/cobradocs/ --vitess-dir "/Users/matt/git/vitess" --version-pairs "main:22.0" mysqlctl
COMMIT_HASH=7386140b97; \
    	git add -u content && \
    	if ! git diff --cached --quiet HEAD --; then \
    	  git commit -s -m "Update cobradocs for $COMMIT_HASH for mysqlctl"; \
    	fi
[rohit/slimmer-docs fb7ce160] Update cobradocs for 7386140b97 for mysqlctl
 7 files changed, 7 insertions(+), 7 deletions(-)
...

Copy link
Collaborator

@mattlord mattlord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than what I noted, it works great! Thank you for taking the time.

I'm approving so that you can merge when you've been able to address the noted issue(s) as you feel best.

@rohit-nayak-ps
Copy link
Contributor Author

I'm approving so that you can merge when you've been able to address the noted issue(s) as you feel best.

When we sync the docs for older versions we checkout the previous releases and if we use go 1.23 we get the invalid reference to runtime.roundupsize error which is the reason for the failure. We need to use go 1.21 while generating docs for older versions.

I will add that check to the Makefile target.

Also we need to merge the backports from vitessio/vitess#17444 before we can test this. Will move to draft for now.

@rohit-nayak-ps rohit-nayak-ps marked this pull request as draft December 30, 2024 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants