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

Update workflows to support fork PRs #3544

Merged
merged 12 commits into from
Sep 29, 2023
13 changes: 5 additions & 8 deletions .github/workflows/checkboxes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
name: Pull Request Checkboxes

on:
pull_request:
pull_request_target:
types:
- opened
- synchronize
Expand All @@ -14,17 +14,14 @@ concurrency:
group: ${{ github.event.pull_request.number }}-pr-checkboxes
cancel-in-progress: true

permissions:
contents: "read"
pull-requests: "write"

jobs:
pr-checkboxes:
name: Check PR checkboxes

permissions:
contents: "read"
id-token: "write"
pull-requests: "write"

runs-on: ubuntu-latest

steps:
- name: Checkout repository
uses: actions/checkout@v4
Expand Down
316 changes: 316 additions & 0 deletions .github/workflows/contrib/checks.yml
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 a bit worried about maintainabillity here.

How much worse would it be to add a workflow_call parameter such as:

      CONTRIB:
        type: boolean
        required: false
        default: true

And then parameterizing the job instead of forking.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried it, and found it to be significantly worse. These if checks get lost in the mass of yaml. I think the duplication makes this slightly more maintainable.

Original file line number Diff line number Diff line change
@@ -0,0 +1,316 @@
name: "Checks: Lints, Tests, Docs"

on:
workflow_call:
inputs:
CONCURRENCY:
required: true
type: string
PR_NUMBER:
required: false
type: string
default: ""

concurrency:
group: ${{ inputs.CONCURRENCY }}-checks
cancel-in-progress: true

env:
PYTHON_VERSION: "3.8"
# web_sys_unstable_apis is required to enable the web_sys clipboard API which egui_web uses
# https://rustwasm.github.io/wasm-bindgen/api/web_sys/struct.Clipboard.html
# https://rustwasm.github.io/docs/wasm-bindgen/web-sys/unstable-apis.html
RUSTFLAGS: --cfg=web_sys_unstable_apis --deny warnings

# See https://github.com/ericseppanen/cargo-cranky/issues/8
RUSTDOCFLAGS: --deny warnings --deny rustdoc::missing_crate_level_docs

permissions:
contents: "read"

jobs:
py-lints:
name: Python lints (black, mypy, flake8)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || github.sha }}

- uses: extractions/setup-just@v1
with:
just-version: 1.5

- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: "3.11"

- name: Install Python dependencies
run: |
pip install --upgrade pip
pip install -r rerun_py/requirements-lint.txt

- name: Lint Python
run: |
just py-lint

- name: Check requirements
run: |
just py-requirements

py-test-docs:
name: Test Python Docs
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || github.sha }}

- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: "3.8"

- name: Install Python dependencies
run: |
pip install --upgrade pip
pip install -r rerun_py/requirements-doc.txt

- name: Build via mkdocs
run: |
mkdocs build -f rerun_py/mkdocs.yml

no-codegen-changes:
name: Check if running codegen would produce any changes
runs-on: ubuntu-latest-16-cores
container:
image: rerunio/ci_docker:0.10.0
steps:
# Note: We explicitly don't override `ref` here. We need to see if changes would be made
# in a context where we have merged with main. Otherwise we might miss changes such as one
# PR introduces a new type and another PR changes the codegen.
- uses: actions/checkout@v4

- name: Codegen
uses: actions-rs/cargo@v1
with:
command: codegen

- name: No Diffs From Running Codegen
run: |
git diff --exit-code

rs-lints:
name: Rust lints (fmt, check, cranky, tests, doc)
runs-on: ubuntu-latest-16-cores
container:
image: rerunio/ci_docker:0.10.0
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || github.sha }}

# First do our check with --locked to make sure `Cargo.lock` is up to date
- name: Check all features
uses: actions-rs/cargo@v1
with:
command: check
args: --locked --all-features

- name: Rustfmt
uses: actions-rs/cargo@v1
with:
command: fmt
args: --all -- --check

- name: Cranky
uses: actions-rs/cargo@v1
with:
command: cranky
args: --all-targets --all-features -- --deny warnings

# Check a few important permutations of the feature flags for our `rerun` library:
- name: Check rerun with `--no-default-features``
uses: actions-rs/cargo@v1
with:
command: cranky
args: --locked -p rerun --no-default-features

- name: Check rerun with `--features sdk`
uses: actions-rs/cargo@v1
with:
command: cranky
args: --locked -p rerun --no-default-features --features sdk

- name: Test doc-tests
uses: actions-rs/cargo@v1
with:
command: test
args: --doc --all-features

- name: cargo doc
uses: actions-rs/cargo@v1
with:
command: doc
args: --no-deps --all-features --workspace

- name: cargo doc --document-private-items
uses: actions-rs/cargo@v1
with:
command: doc
args: --document-private-items --no-deps --all-features --workspace

# Just a normal `cargo test` should always work:
- name: cargo test --all-targets
uses: actions-rs/cargo@v1
with:
command: test

# Full test of everything:
- name: cargo test --all-targets --all-features
uses: actions-rs/cargo@v1
with:
command: test
args: --all-targets --all-features

rs-check-wasm:
name: Check Rust web build (wasm32 + wasm-bindgen)
runs-on: ubuntu-latest-16-cores
container:
image: rerunio/ci_docker:0.10.0
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || github.sha }}

- name: clippy check re_viewer wasm32
run: ./scripts/clippy_wasm.sh

- name: Check re_renderer examples wasm32
uses: actions-rs/cargo@v1
with:
command: check
args: --locked --target wasm32-unknown-unknown --target-dir target_wasm -p re_renderer --examples

- name: Build web-viewer (debug)
uses: actions-rs/cargo@v1
with:
command: run
args: --locked -p re_build_web_viewer -- --debug

toml-lints:
name: Lint TOML files
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || github.sha }}

- name: Install taplo-cli
uses: taiki-e/install-action@v2
with:
tool: taplo-cli

- name: Taplo check
run: |
taplo fmt --check

misc-rerun-lints:
name: Rerun lints
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || github.sha }}

- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: "3.8"

- name: Install dependencies
run: |
pip install gitignore_parser python-frontmatter
pip install -r ./scripts/ci/requirements.txt

- name: Rerun lints
run: |
./scripts/lint.py

- name: Check for too large files
run: |
./scripts/ci/check_large_files.sh

- name: Check Python example requirements
run: |
./scripts/ci/check_requirements.py

- name: Check Python example thumbnails
run: |
./scripts/ci/thumbnails.py check

spell-check:
name: Spell Check
runs-on: ubuntu-latest
steps:
- name: Checkout Actions Repository
uses: actions/checkout@v4
with:
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || github.sha }}

- name: Check spelling of entire workspace
uses: crate-ci/typos@master

rs-cargo-deny:
name: Cargo Deny
runs-on: ubuntu-latest
container:
image: rerunio/ci_docker:0.10.0
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || github.sha }}

- name: Cargo Deny
shell: bash
id: expected_version
run: ./scripts/ci/cargo_deny.sh

cpp-formating:
name: C++ formatting check
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || github.sha }}

- name: Run clang format on all relevant files
uses: jidicula/[email protected]
with:
clang-format-version: "16"
# Only check c/cpp/h/hpp (default checks also .proto and others)
include-regex: ^.*\.(c|cpp|h|hpp)$

cpp-tests:
name: C++ tests
runs-on: ubuntu-latest
container:
image: rerunio/ci_docker:0.10.0
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || github.sha }}

- name: Build and run C++ minimal example
shell: bash
run: ./examples/cpp/minimal/build_and_run.sh --werror

- name: Build and run rerun_cpp tests
shell: bash
run: ./rerun_cpp/build_and_run_tests.sh --werror

- name: Build code examples
shell: bash
run: ./docs/code-examples/build_all.sh --werror

Loading
Loading