Add github action to sync api docs to the user docs site #276

tinygrasshopper · 2024-05-16T07:43:34Z

Currently this generates references docs, based on the tags supplied in the open api docs
Will add altering if the job fails as a separate PR

jgresty

Couple of things:

There is no linting or build step so we wont know if anything is broken until we try to run it next
Documentation could be far better. Even a basic what this does and why in a readme would be appreciated
There is a lot of behaviour here that is untested. I would not be comfortable updating dependencies etc

.github/workflows/sync-api-docs.yml

tools/api-docs-generator/spec_fetcher_test.go

jgresty · 2024-05-16T12:37:16Z

tools/api-docs-generator/main.go

+
+func main() {
+	if len(os.Args) != 3 {
+		log.Fatal("usage: api-docs <config-file> <docs-dir>")


Perhaps consider https://github.com/spf13/viper , although this is fine for now

tools/api-docs-generator/reference_docs.go

tools/api-docs-generator/main.go

jgresty · 2024-05-16T13:31:48Z

Oh and alerting, how will we know if this fails?

jgresty

This is looking good!

I'm still concerned what will happen if this script runs before a previous pull request is merged, looks like it is using the same branch name every time so will it error? Stack empty commits? No-op? (hopefully the last one)

I can see there were a lot of notes that were previously on Apiary that have been lost on the new V1 docs. Eg: https://snyk.docs.apiary.io/#reference/import-projects . Some of those notes may be redundant but we are losing information in this new system. However the one for licenses is present: https://snyk.docs.apiary.io/#reference/licenses . Was that one manually added? Will this script overwrite it?

jgresty · 2024-05-22T11:56:30Z

.github/workflows/sync-api-docs.yml

+    steps:
+      - uses: actions/checkout@v4
+        with:
+          #TODO: Remove before merge


Need to do this

jgresty · 2024-05-22T11:58:30Z

.github/workflows/sync-api-docs.yml

+          make run | tee /tmp/run.log
+          result_code=${PIPESTATUS[0]}
+          echo 'MENU<<EOF' >> $GITHUB_OUTPUT
+          cat /tmp/run.log >> $GITHUB_OUTPUT
+          echo 'EOF' >> $GITHUB_OUTPUT
+          exit $result_code


Can't you write to $GITHUB_OUTPUT directly? I don't see why /tmp/run.log is necessary

echo 'MENU<<EOF' >> $GITHUB_OUTPUT make run >> $GITHUB_OUTPUT echo 'EOF' >> $GITHUB_OUTPUT

Looses the exit status of the run; it will return the exit status of echo

echo MENU >> $GITHUB_OUTPUT make run >> $GITHUB_OUTPUT

?

Or

set -o pipefail echo MENU >> $GITHUB_OUTPUT make run | tee -a $GITHUB_OUTPUT

if you want to preserve stdout

The way multi line strings work in GHA needs the closing delimiter: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings

jgresty · 2024-05-22T12:02:24Z

.github/workflows/test-generator.yml

+    steps:
+      - uses: actions/checkout@v4
+        with:
+          #TODO: Remove before merge


Another todo

jgresty · 2024-05-22T12:03:45Z

tools/api-docs-generator/Makefile

+test: tidy lint
+	go test ./...
+
+run:
+	@go run . config.yml ../..
+
+lint: tidy
+	go run github.com/golangci/golangci-lint/cmd/golangci-lint run
+
+format: tidy
+	go run golang.org/x/tools/cmd/goimports -w -local=github.com/snyk/user-docs/tools/api-docs-generator .
+
+tidy:
+	go mod tidy


nit: all of these should be marked .PHONY as they do not output the target

jgresty · 2024-05-22T12:04:43Z

tools/api-docs-generator/Makefile

+	@go run . config.yml ../..
+
+lint: tidy
+	go run github.com/golangci/golangci-lint/cmd/golangci-lint run


Please pin the version, there are occasionally breaking changes

version is pinned in the go.mod file

jgresty · 2024-05-22T12:10:45Z

tools/api-docs-generator/fetcher/spec_fetcher.go

+	}
+
+	formattedSpec := bytes.NewBufferString("")
+	err = json.Indent(formattedSpec, jsonSpec, "", "  ")


json keys are unordered, I'm not sure if vervet generation is stable if content is the same but something else changes that is unrelated to the current spec - eg a new version is published

interesting point, have not seen this occur yet, keep on eye on the PR if these PRs are generated

jgresty · 2024-05-22T12:12:06Z

tools/api-docs-generator/fetcher/spec_fetcher.go

+func getLatestGAVersion(versions []string) string {
+	gaVersions := []string{}
+	for _, version := range versions {
+		if !strings.Contains(version, "~") {


~ga is an optional valid suffix for ga versions

the suffix is used for version resolution right? When listing the versions, I dont see us rendering ~ga at the end of versions

jgresty · 2024-05-22T12:13:16Z

tools/api-docs-generator/fetcher/spec_fetcher.go

+			gaVersions = append(gaVersions, version)
+		}
+	}
+	sort.Strings(gaVersions)


You're already iterating over the entire list above, seems very inefficient to do that and then sort them after to select one element when you can select the largest element in one pass

jgresty · 2024-05-22T12:49:15Z

.github/workflows/sync-api-docs.yml

+          body: |
+            This PR was automatically generated by the API docs synchronization action. Please review the changes and merge if they look good.
+            ```
+            ${{ steps.generate.outputs.MENU }}


I think we need to have a bit more explanation about what to do with this menu in the pull request. At the moment it is tribal knowledge that can easily be misunderstood.

tinygrasshopper · 2024-05-22T18:54:10Z

I can see there were a lot of notes that were previously on Apiary that have been lost on the new V1 docs.

Yep, will add that feature as a followup PR

I'm still concerned what will happen if this script runs before a previous pull request is merged

The default behaviour of the PR action is to merge it into the previous PR

mikeromard

Approving on behalf of docs team.

tinygrasshopper requested a review from a team as a code owner May 16, 2024 07:43

tinygrasshopper force-pushed the chore/docs-action branch 11 times, most recently from 5e9b091 to 4d7d596 Compare May 16, 2024 08:44

tinygrasshopper requested a review from jgresty May 16, 2024 12:15

jgresty reviewed May 16, 2024

View reviewed changes

tinygrasshopper force-pushed the chore/docs-action branch from 4d7d596 to 302d6a1 Compare May 17, 2024 09:40

tinygrasshopper changed the title ~~Add github action to sync api docs to the user docs site~~ [wip] Add github action to sync api docs to the user docs site May 21, 2024

tinygrasshopper force-pushed the chore/docs-action branch 11 times, most recently from c958691 to 43e14cb Compare May 22, 2024 10:33

tinygrasshopper changed the title ~~[wip] Add github action to sync api docs to the user docs site~~ Add github action to sync api docs to the user docs site May 22, 2024

tinygrasshopper force-pushed the chore/docs-action branch from 43e14cb to fda4cac Compare May 22, 2024 10:41

tinygrasshopper requested a review from jgresty May 22, 2024 11:55

jgresty approved these changes May 22, 2024

View reviewed changes

Add github action to sync api docs to the user docs site

3f75aef

tinygrasshopper force-pushed the chore/docs-action branch from fda4cac to 3f75aef Compare May 22, 2024 20:05

mikeromard approved these changes May 23, 2024

View reviewed changes

tinygrasshopper merged commit e9ca07b into main May 23, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add github action to sync api docs to the user docs site #276

Add github action to sync api docs to the user docs site #276

tinygrasshopper commented May 16, 2024 •

edited

Loading

jgresty left a comment

jgresty May 16, 2024

jgresty commented May 16, 2024

jgresty left a comment

jgresty May 22, 2024

jgresty May 22, 2024

tinygrasshopper May 22, 2024

jgresty May 22, 2024

jgresty May 22, 2024

tinygrasshopper May 22, 2024

jgresty May 22, 2024

jgresty May 22, 2024

jgresty May 22, 2024

tinygrasshopper May 22, 2024

jgresty May 22, 2024

tinygrasshopper May 22, 2024

jgresty May 22, 2024

tinygrasshopper May 22, 2024

jgresty May 22, 2024

jgresty May 22, 2024

tinygrasshopper commented May 22, 2024 •

edited

Loading

mikeromard left a comment

Add github action to sync api docs to the user docs site #276

Add github action to sync api docs to the user docs site #276

Conversation

tinygrasshopper commented May 16, 2024 • edited Loading

jgresty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgresty commented May 16, 2024

jgresty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tinygrasshopper commented May 22, 2024 • edited Loading

mikeromard left a comment

Choose a reason for hiding this comment

tinygrasshopper commented May 16, 2024 •

edited

Loading

tinygrasshopper commented May 22, 2024 •

edited

Loading