Skip to content

fix CLI reference generation for current and next versions #811

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.

Sign up for GitHub

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

Merged
jetstack-bot merged 2 commits into from
Jan 26, 2022
Merged

fix CLI reference generation for current and next versions #811

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd5c24a
break out CLI generation into separate function
jahrlin Jan 26, 2022
1c28402
improve generate script documentation comments
jahrlin Jan 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 24 additions & 21 deletions scripts/gendocs/generate
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,20 @@ gencliversions="${1:-0}"
genversion() {
checkout "$1"
gendocs "$2"
}

genversionwithcli() {
genversion "$1" "$2"

if [ "$gencliversions" == 1 ]; then
genclireference "$2" "cmd/acmesolver" "acmesolver"
genclireference "$2" "cmd/cainjector" "cainjector"
genclireference "$2" "cmd/ctl" "cmctl"
genclireference "$2" "cmd/controller" "controller"
genclireference "$2" "cmd/webhook" "webhook"

# if any of the above steps succeeded copy over the index file
if [ -d "$REPO_ROOT/content/en/$2/cli" ]; then
cp "$REPO_ROOT/content/en/docs/cli/_index.md" "${REPO_ROOT}/content/en/$2/cli/"
fi
genclireference "$2" "cmd/acmesolver" "acmesolver"
genclireference "$2" "cmd/cainjector" "cainjector"
genclireference "$2" "cmd/ctl" "cmctl"
genclireference "$2" "cmd/controller" "controller"
genclireference "$2" "cmd/webhook" "webhook"

# if any of the above steps succeeded copy over the index file
if [ -d "$REPO_ROOT/content/en/$2/cli" ]; then
cp "$REPO_ROOT/content/en/docs/cli/_index.md" "${REPO_ROOT}/content/en/$2/cli/"
fi
}

Expand Down Expand Up @@ -142,21 +144,22 @@ $output
EOF
}

genclireference "docs" "cmd/acmesolver" "acmesolver"
genclireference "docs" "cmd/cainjector" "cainjector"
genclireference "docs" "cmd/ctl" "cmctl"
genclireference "docs" "cmd/controller" "controller"
genclireference "docs" "cmd/webhook" "webhook"

# The branches named here exist in the `jetstack/cert-manager` repo.

genversion "release-1.7" "next-docs"
genversionwithcli "release-1.7" "next-docs"

# In cert-manager 1.6 cert-manager.io and acme.cert-manager.io alpha and beta
# API versions are no longer served, but the apis are still in the codebase. We
# don't want to show the docs for those API versions so this is a workaround.
# This will only be necessary for the release-1.6 branch.
checkout "release-1.6"

@wallrj wallrj Jan 26, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This checkout and the comment above relate to the rm and gendocs command below,
so I think it the genclireference commands should be moved after those.

@jahrlin jahrlin Jan 26, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, I have moved the comment.

genclireference must happen before we delete the legacy API versions because the code does not compile without them. I added a comment on that as well


genclireference "docs" "cmd/acmesolver" "acmesolver"
genclireference "docs" "cmd/cainjector" "cainjector"
genclireference "docs" "cmd/ctl" "cmctl"
genclireference "docs" "cmd/controller" "controller"
genclireference "docs" "cmd/webhook" "webhook"

@wallrj wallrj Jan 26, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And if these lines are deliberately placed after the checkout command, then I'd add a comment explaining why it's important, so that someone doesn't come alone and rearrange this code.

@jahrlin jahrlin Jan 26, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have added a comment here covering this!


rm -r pkg/apis/acme/v1alpha2
rm -r pkg/apis/acme/v1alpha3
rm -r pkg/apis/acme/v1beta1
Expand All @@ -174,9 +177,9 @@ rm -r pkg/apis/certmanager/v1alpha3
rm -r pkg/apis/certmanager/v1beta1
gendocs "v1.6-docs"

# when adding a new version here run
# ./scripts/gendocs/generate 1 to force generation of CLI docs
# and add them to the git index
# when adding a new version here use
# genversionwithcli to force generation of CLI docs
# and add them to the git index, then switch back to genversion

@wallrj wallrj Jan 26, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand this comment.
Does it mean that I should add another genversionwithcli line somewhere in the scipt.
Please add a little more detail about how this script should be modified in future.

@jahrlin jahrlin Jan 26, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have re-formulated the comment and included an example for the next change in this section

genversion "release-1.5" "v1.5-docs"
genversion "release-1.4" "v1.4-docs"
genversion "release-1.3" "v1.3-docs"
Expand Down