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.

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

feat(docs): add workflow to generate documentation on PR merge #12681

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
+36 −3
Open

feat(docs): add workflow to generate documentation on PR merge #12681

Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
217645a
feat(docs): add workflow to generate documentation on PR merge
virajbhartiya Nov 7, 2024
84e5487
Update .github/workflows/docsgen-cli.yml
virajbhartiya Nov 7, 2024
145b3b4
remove docsgen-cli check in github actions
virajbhartiya Nov 7, 2024
6f3602a
run on push
virajbhartiya Nov 7, 2024
5bac04d
Merge branch 'master' into docsgen-workflow
virajbhartiya Nov 7, 2024
5a6d6eb
address changes
virajbhartiya Nov 18, 2024
2cceea8
Refactor workflow to check and generate documentation
virajbhartiya Nov 18, 2024
046880f
update
virajbhartiya Nov 18, 2024
190f7cb
update
virajbhartiya Nov 18, 2024
938f8b5
update
virajbhartiya Nov 18, 2024
4e5a8c6
add link for bot configuration discussion
virajbhartiya Nov 18, 2024
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
39 changes: 36 additions & 3 deletions .github/workflows/check.yml → .github/workflows/check-and-gen.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Check
name: Check and Generate

on:
pull_request:
Expand Down Expand Up @@ -33,8 +33,6 @@ jobs:
- uses: ./.github/actions/make-deps
- run: make gen
Copy link
Member

Choose a reason for hiding this comment

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

Probably too big a leap for now, but I'd actually like to move docsgen out of make gen so we can run it here. Maybe for now we just do docsgen-cli and if it works OK we'll consider expanding it.

- run: git diff --exit-code
- run: make docsgen-cli
- run: git diff --exit-code
check-lint:
name: Check (lint-all)
runs-on: ubuntu-latest
Expand Down Expand Up @@ -70,3 +68,38 @@ jobs:
- uses: ./.github/actions/install-go
- run: go mod tidy -v
- run: git diff --exit-code
generate-docs:
name: Generate Documentation
runs-on: ubuntu-latest
if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/heads/release/'))
Copy link
Member

Choose a reason for hiding this comment

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

I'm unsure about this and whether a merge into one of these branches would trigger the job as well, we only want it to run on the PR branch, strictly. So this line worries me a little and it needs verification, somehow, or we need a subject-matter expert to help.

Copy link
Member

Choose a reason for hiding this comment

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

@BigLep maybe something to pull IPDX into to help review.

Copy link
Member

Choose a reason for hiding this comment

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

@galargh : do you know offhand whether this will only run on pushes to PR branches and not on master or release branches?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's the opposite of what this conditional says.

The conditional is: accept a workflow run which was triggered by a push event to either master or release/* branch.

permissions:
contents: write
steps:
- uses: actions/checkout@v4
with:
submodules: 'recursive'
fetch-depth: 0

- uses: ./.github/actions/install-system-dependencies
- uses: ./.github/actions/install-go
- uses: ./.github/actions/make-deps

- name: Generate API documentation using docsgen-cli
run: |
make docsgen-cli || {
echo "Error: Documentation generation failed"
exit 1
}

- name: Commit and push if documentation changed
run: |
if [ -n "$(git status --porcelain)" ]; then
# Bot email is configured from: https://github.com/orgs/community/discussions/26560
git config user.name "github-actions[bot]"
git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
virajbhartiya marked this conversation as resolved.
Show resolved Hide resolved
git add .
git commit -m "docs: update API documentation via docsgen-cli"
BigLep marked this conversation as resolved.
Show resolved Hide resolved
git push
Copy link
Member

Choose a reason for hiding this comment

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

Another thing I'm wondering is if this is going to work to fork-branches, if this is run a a filecoin-project GHA runner, will we always have permissions to push back into the PR branch? Do we need to ensure we have a "can edit branches" thing turned on in our repo? Do we need to do anything else to make this work?

Copy link
Contributor

Choose a reason for hiding this comment

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

As currently configured, this will be run on push events to master or release/*. But it won't work because they are protected. To overcome this, one would have to use a personal access token for a user that can disregard the branch protection rules.

else
echo "No documentation changes to commit"
fi