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: DOC-228: Docs PR preview action #1107

Merged
merged 21 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
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
36 changes: 36 additions & 0 deletions .github/actions/get-changes/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Note this expects checkout to already be performed
name: 'Get changed plugins'
description: 'Get the list of plugins that have been modified in the current PR or push'
outputs:
packages:
description: 'Changed packges'
value: ${{ steps.filter.outputs.changes }}

runs:
using: 'composite'
steps:
- name: Filter paths
uses: dorny/paths-filter@v3
id: filter
with:
filters: |
plotly-express:
- plugins/plotly-express/**
- .github/workflows/test-*.yml
- sphinx_ext/*
matplotlib:
- plugins/matplotlib/**
- .github/workflows/test-*.yml
json:
- plugins/json/**
- .github/workflows/test-*.yml
ui:
- plugins/ui/**
- .github/workflows/test-*.yml
- sphinx_ext/*
utilities:
- plugins/utilities/**
- .github/workflows/test-*.yml
packaging:
- plugins/packaging/**
- .github/workflows/test-*.yml
73 changes: 73 additions & 0 deletions .github/workflows/build-docs.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
name: Build Docs

on:
workflow_call:
inputs:
package:
description: The plugin to publish the docs for
required: true
type: string
version:
description: The version of the plugin to publish the docs for
type: string
default: 'main'

jobs:
check-make-docs:
runs-on: ubuntu-24.04
outputs:
files_exists: ${{ steps.check_files.outputs.files_exists }}
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Check file existence
id: check_files
uses: andstor/file-existence-action@v3
with:
files: 'plugins/${{ inputs.package }}/make_docs.py'

build-plugin:
if: needs.check-make-docs.outputs.files_exists == 'true'
needs: check-make-docs
uses: ./.github/workflows/build-python-package.yml
with:
package: ${{ inputs.package }}
js: false # No need for JS to build docs
artifact-name: 'dist-docs-${{ inputs.package }}' # Don't collide with Python publishing action

build-docs:
needs: build-plugin
runs-on: ubuntu-24.04
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Download dist
uses: actions/download-artifact@v4
with:
name: dist-docs-${{ inputs.package }}
path: plugins/${{ inputs.package }}/dist/

- name: Install requirements
run: pip install -r sphinx_ext/sphinx-requirements.txt

- name: Install wheel
run: pip install plugins/${{ inputs.package }}/dist/*.whl

- name: Run make_docs.py
run: python plugins/${{ inputs.package }}/make_docs.py

# Since the upload happens after any changed docs build, we'll setup so
# the artifact can be downloaded and merged into 1 directory structure that matches
# the expected structure in the GCP bucket
- name: Set upload structure
run: |
mkdir -p docs-build/${{ inputs.package }}/${{ inputs.version }}
cp -r plugins/${{ inputs.package }}/docs/build/markdown/* docs-build/${{ inputs.package }}/${{ inputs.version }}

- name: Upload docs
uses: actions/upload-artifact@v4
with:
name: docs-build-${{ inputs.package }}-${{ inputs.version }} # This will be downloaded by publish-docs.yml
path: docs-build
82 changes: 0 additions & 82 deletions .github/workflows/build-main-docs.yml

This file was deleted.

23 changes: 18 additions & 5 deletions .github/workflows/build-python-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ on:
workflow_call:
inputs:
package:
description: 'Name of the package to build'
required: true
type: string
js:
description: 'Whether to build and bundle the JS package with Python'
required: false
type: boolean
default: true
artifact-name:
description: 'Name of the artifact to upload'
required: true
type: string

Expand All @@ -14,31 +24,34 @@ jobs:
- name: Checkout
uses: actions/checkout@v4

- name: Check file existence
- name: Check JS existence
if: ${{ inputs.js == 'true' }}
Copy link
Member

Choose a reason for hiding this comment

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

Why is this checking against the string 'true' instead of just checking inputs.js ? Seems like this should be a boolean based on the type, not a string.
https://docs.github.com/en/actions/sharing-automations/creating-actions/metadata-syntax-for-github-actions#inputs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The boolean flag is a lie (or at least was and Github just hangs when I try to load more to see what the actual resolution of this ticket was) See here

I'm 99% sure it's still a string value and the boolean just helps with documentation or for manual dispatch actions it shows a checkbox.

id: check_files
uses: andstor/file-existence-action@v3
with:
files: 'plugins/${{ inputs.package }}/src/js/package.json'

- name: Setup Node
if: steps.check_files.outputs.files_exists == 'true'
id: setup-node
if: ${{ inputs.js == 'true' && steps.check_files.outputs.files_exists == 'true' }}
Copy link
Member

Choose a reason for hiding this comment

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

Though I see this checks .files_exists against 'true' as well, and that matches the documentation: https://github.com/andstor/file-existence-action
Don't get why it's not a bool...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It actually turns inputs/outputs into environment variables, so they're always strings

uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'
cache: 'npm'

- name: Install npm dependencies
if: steps.check_files.outputs.files_exists == 'true'
if: ${{ steps.setup-node.conclusion == 'success'}}
run: npm ci

- name: Build npm packages
if: steps.check_files.outputs.files_exists == 'true'
if: ${{ steps.setup-node.conclusion == 'success'}}
run: npm run build -- --scope "@deephaven/js-plugin-${{ inputs.package }}"

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.x'
cache: 'pip'

- name: Install build dependencies
run: python -m pip install --upgrade setuptools wheel build
Expand All @@ -49,6 +62,6 @@ jobs:
- name: Upload dist
uses: actions/upload-artifact@v4
with:
name: dist-${{ inputs.package }}
name: ${{ inputs.artifact-name }}
path: plugins/${{ inputs.package }}/dist/
if-no-files-found: error
68 changes: 0 additions & 68 deletions .github/workflows/make-docs.yml

This file was deleted.

40 changes: 9 additions & 31 deletions .github/workflows/modified-plugin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,36 +23,14 @@ jobs:
permissions:
pull-requests: read
outputs:
packages: ${{ steps.filter.outputs.changes }}
packages: ${{ steps.filter.outputs.packages }}
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Filter paths
uses: dorny/paths-filter@v3
uses: ./.github/actions/get-changes
id: filter
with:
filters: |
plotly-express:
- plugins/plotly-express/**
- .github/workflows/test-*.yml
- sphinx_ext/*
matplotlib:
- plugins/matplotlib/**
- .github/workflows/test-*.yml
json:
- plugins/json/**
- .github/workflows/test-*.yml
ui:
- plugins/ui/**
- .github/workflows/test-*.yml
- sphinx_ext/*
utilities:
- plugins/utilities/**
- .github/workflows/test-*.yml
packaging:
- plugins/packaging/**
- .github/workflows/test-*.yml

# Test all python packages that have been modified individually
test-python:
Expand All @@ -77,7 +55,7 @@ jobs:
test-results:
if: ${{ always() }}
runs-on: ubuntu-22.04
needs: [test-python, test-js]
needs: [test-python, test-js, build-docs]
steps:
# Only fail if one of the previous tests failed
- run: exit 1
Expand All @@ -102,16 +80,16 @@ jobs:
with:
package: ${{ needs.filter-release.outputs.package }}

# Main docs are built here, whereas release docs are built in release-python-package.yml
build-main-docs:
# Main and PR docs are built here, whereas release docs are built in release-python-package.yml
build-docs:
needs: changes
# Only build main docs for push on main branch and PRs to main
if: ${{ needs.changes.outputs.packages != '[]' && needs.changes.outputs.packages != '' && (github.ref == 'refs/heads/main' || github.base_ref == 'main') && (github.event_name == 'push' || github.event_name == 'pull_request')}}
# Only build main docs for push on main branch and any PRs
if: ${{ needs.changes.outputs.packages != '[]' && needs.changes.outputs.packages != '' && (github.ref == 'refs/head/main' || github.event_name == 'pull_request') }}
strategy:
matrix:
package: ${{ fromJSON(needs.changes.outputs.packages) }}
uses: ./.github/workflows/build-main-docs.yml
uses: ./.github/workflows/build-docs.yml
secrets: inherit
with:
package: ${{ matrix.package }}
event_name: ${{ github.event_name }}
version: ${{ github.event_name == 'pull_request' && format('pr-{0}', github.event.number) || github.ref == 'refs/head/main' && 'main' }}
Loading
Loading