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

Add Workflow and update cli documentation #6414

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

grahamplata
Copy link
Contributor

@grahamplata grahamplata commented Jan 14, 2025

Update GitHub docs workflow to check if there were cli help menu changes.

@grahamplata grahamplata self-assigned this Jan 14, 2025
@grahamplata grahamplata added the Type:Documentation Improvements or additions to documentation label Jan 14, 2025
@grahamplata grahamplata marked this pull request as ready for review January 15, 2025 15:30
cli/README.md Outdated
@@ -31,3 +31,5 @@ go run ./cli start dev-project
## Generating CLI reference docs

See `../docs/README.md` for details about the generated CLI reference docs.

dummy cli change
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember to remove this :)

title: Dummy
---

# Dummy
Copy link
Contributor

Choose a reason for hiding this comment

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

And this

Comment on lines 40 to 44
- name: Check if CLI docs need to be generated
if: ${{ steps.filter.outputs.cli == 'true' }}
run: |
echo "Changes detected in cli, generating docs"
make docs.generate
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid magic where the published docs do not match the contents of the docs directory on main, can we fail CI with an informative error if make docs.generate leads to changed files (you should be able to use Git to check if it leads to uncommitted changes)?

It's an extra manual step for the developer to run and commit make docs.generate, but it'll be good to have eyes on that during review anyway.

Comment on lines +40 to +43
- name: Check if new files were created
if: ${{ steps.filter.outputs.cli == 'true' }}
run: |
git diff --exit-code || (echo "New files were created, please run 'make docs.generate' locally and commit the changes" && exit 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this missing the actual make docs.generate? Like:

Suggested change
- name: Check if new files were created
if: ${{ steps.filter.outputs.cli == 'true' }}
run: |
git diff --exit-code || (echo "New files were created, please run 'make docs.generate' locally and commit the changes" && exit 1)
- name: Check if new files were created
if: ${{ steps.filter.outputs.cli == 'true' }}
run: |
make docs.generate
git diff --exit-code || (echo "New files were created, please run 'make docs.generate' locally and commit the changes" && exit 1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type:Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants