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

ci: Add a CI to check documentation changes #943

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

haytok
Copy link
Member

@haytok haytok commented May 14, 2024

This fix introduces a new CI to check whether the command reference documentations need to be updated.

When new commands or options are added, the
finch gen-docs generate -p cmd/finch/ command should be run locally to update the documentation.

However, it's easy to forget this step.

An issue has also been created that proposes to detect these differences.

Therefore, in this fix, the new CI step will generate the documentation and compare it with the existing files.
If there are differences, the CI will fail, prompting contributors to update the documentations.

Issue #, if available: #942

Description of changes: The details are described in the commit message.

Testing done: N/A

  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

This fix introduces a new CI to check whether the command reference
documentations need to be updated.

When new commands or options are added, the
`finch gen-docs generate -p cmd/finch/` command should be run locally to
update the documentation.

However, it's easy to forget this step.

An issue has also been created that proposes to detect these differences.

- runfinch#942

Therefore, in this fix, the new CI step will generate the documentation
and compare it with the existing files.
If there are differences, the CI will fail, prompting contributors to
update the documentations.

Signed-off-by: Hayato Kiwata <haytok@amazon.co.jp>
@haytok haytok requested a review from a team as a code owner May 14, 2024 00:06
@haytok
Copy link
Member Author

haytok commented May 14, 2024

The newly added CI is failing.

Details

0s
Run git add -N docs/cmd
  git add -N docs/cmd
  git diff --exit-code docs/cmd
  shell: /bin/bash -e {0}
diff --git a/docs/cmd/finch_build.md b/docs/cmd/finch_build.md
index 022[1](https://github.com/runfinch/finch/actions/runs/9071519919/job/24925389774?pr=943#step:9:1)3df..e69de29 100644
--- a/docs/cmd/finch_build.md
+++ b/docs/cmd/finch_build.md
@@ -1,3[2](https://github.com/runfinch/finch/actions/runs/9071519919/job/24925389774?pr=943#step:9:2) +0,0 @@
-# finch build
-
-Build an image from a Dockerfile. Needs buildkitd to be running.
-If Dockerfile is not present and -f is not specified, it will look for Containerfile and build with it.
-
-```text
-finch build [flags] PATH
-```
-
-## Options
-
-```text
-      --build-arg stringArray    Set build-time variables
-      --buildkit-host string     BuildKit address [$BUILDKIT_HOST] (default "unix:///run/buildkit/buildkitd.sock")
-      --cache-from stringArray   External cache sources (eg. user/app:cache, type=local,src=path/to/dir)
-      --cache-to stringArray     Cache export destinations (eg. user/app:cache, type=local,dest=path/to/dir)
-  -f, --file string              Name of the Dockerfile
-  -h, --help                     help for build
-      --iidfile string           Write the image ID to the file
-      --label stringArray        Set metadata for an image
-      --network string           Set type of network for build (format:network=default|none|host) (default "default")
-      --no-cache                 Do not use cache when building the image
-  -o, --output string            Output destination (format: type=local,dest=path)
-      --platform strings         Set target platform for build (e.g., "amd6[4](https://github.com/runfinch/finch/actions/runs/9071519919/job/24925389774?pr=943#step:9:4)", "arm64")
-      --progress string          Set type of progress output (auto, plain, tty). Use plain to show container output (default "auto")
-  -q, --quiet                    Suppress the build output and print image ID on success
-      --rm                       Remove intermediate containers after a successful build (default true)
-      --secret stringArray       Secret file to expose to the build: id=mysecret,src=/local/secret
-      --ssh stringArray          SSH agent socket or keys to expose to the build (format: default|<id>[=<socket>|<key>[,<key>]])
-  -t, --tag stringArray          Name and optionally a tag in the 'name:tag' format
-      --target string            Set the target build stage to build
-```
Error: Process completed with exit code 1.

This is because there are some outdated documentation files when running make gen-docs locally, such as:

haytok ~/workspace/haytok/finch [add-ci-for-finch-gen-docs-generate-command]
> git status
On branch add-ci-for-finch-gen-docs-generate-command
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   docs/cmd/finch_build.md
	modified:   docs/cmd/finch_container.md
	modified:   docs/cmd/finch_create.md
	modified:   docs/cmd/finch_pull.md
	modified:   docs/cmd/finch_run.md

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	docs/cmd/finch_vm_settings.md

no changes added to commit (use "git add" and/or "git commit -a")

Therefore, the newly added CI is behaving as expected.

Note that the generation of docs/cmd/finch_vm_settings.md was introduced in the following PR.

Question

As for other documentation files (e.g., docs/cmd/finch_build.md), I plan to add them in a separate PR.
Is this approach acceptable?

@Shubhranshu153
Copy link
Contributor

Do all changes requires a documentation change?
If not may be we should have some override

@pendo324
Copy link
Member

Do all changes requires a documentation change? If not may be we should have some override

Only changes to the cmd/finch/ directory, or updates to the lima bundle should trigger documentation updates

@haytok
Copy link
Member Author

haytok commented May 15, 2024

updates to the lima bundle

What are you referring to?
At the moment, we only check if there are any diffs by running finch gen-docs generate -p docs/cmd/.

@haytok haytok self-assigned this Jun 12, 2024
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.

3 participants