-
Notifications
You must be signed in to change notification settings - Fork 12
feat: provide on-pr workflow for all the basics checks that we need #89
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| # ******************************************************************************* | ||
| # Copyright (c) 2025 Contributors to the Eclipse Foundation | ||
| # Copyright (c) 2026 Contributors to the Eclipse Foundation | ||
| # | ||
| # See the NOTICE file(s) distributed with this work for additional | ||
| # information regarding copyright ownership. | ||
|
|
@@ -11,12 +11,190 @@ | |
| # SPDX-License-Identifier: Apache-2.0 | ||
| # ******************************************************************************* | ||
|
|
||
| name: on-pr | ||
| # This is designed as the main entry point for PR checks, for all S-CORE repositories. | ||
|
|
||
| # Attention: this workflow must be called with following permissions: | ||
| # permissions: | ||
| # id-token: write | ||
| # contents: read | ||
|
AlexanderLanin marked this conversation as resolved.
|
||
|
|
||
| # The name will show up as "PR Checl / 🔁 / <workflow-name>", | ||
| # so there is no reason to add any text where the icon is. | ||
| # --- | ||
| # Update: this name is actually not used anywhere! wtf? | ||
| name: 🔁 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use ASCI only names...
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's common practice to use icons in job names, action names etc. In this specific case I'll remove the name completely. It's not used anywhere, and there is no warning when it's not given. Will check the docs. |
||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [main] | ||
| workflow_call: | ||
|
|
||
| jobs: | ||
| score-pr-checks: | ||
| uses: ./.github/workflows/score-pr-checks.yml | ||
| detect-repo-capabilities: | ||
| name: Detect repository capabilities | ||
| runs-on: ${{ vars.REPO_RUNNER_LABELS && fromJSON(vars.REPO_RUNNER_LABELS) || 'ubuntu-24.04' }} | ||
| outputs: | ||
| has_bazel: ${{ steps.detect.outputs.has_bazel }} | ||
| has_bazel_lock: ${{ steps.detect.outputs.has_bazel_lock }} | ||
| has_devcontainer: ${{ steps.detect.outputs.has_devcontainer }} | ||
| has_python_uv: ${{ steps.detect.outputs.has_python_uv }} | ||
| has_precommit: ${{ steps.detect.outputs.has_precommit }} | ||
|
|
||
| permissions: | ||
| id-token: write | ||
| contents: read | ||
|
|
||
| steps: | ||
| - name: 📥 Check out | ||
| uses: actions/checkout@v6 | ||
|
|
||
| - name: 📥 Check out cicd-workflows | ||
| uses: dariocurr/checkout-called@v6 | ||
| with: | ||
| path: .cicd-workflows | ||
|
|
||
| - name: 🕵️ Detect repository capabilities | ||
| id: detect | ||
| run: ./.cicd-workflows/scripts/detect-repo-capabilities.sh "${{ github.workspace }}" | ||
|
|
||
| precommit: | ||
| name: 🧹 Run pre-commit checks | ||
| needs: detect-repo-capabilities | ||
| if: needs.detect-repo-capabilities.outputs.has_precommit == 'true' | ||
| runs-on: ${{ vars.REPO_RUNNER_LABELS && fromJSON(vars.REPO_RUNNER_LABELS) || 'ubuntu-24.04' }} | ||
| steps: | ||
| - name: 📥 Check out | ||
| uses: actions/checkout@v6 | ||
| - name: ⚙️ Setup uv | ||
| uses: astral-sh/setup-uv@v5 | ||
| - name: 🛠️ Run pre-commit | ||
| run: uvx pre-commit run --all-files | ||
|
|
||
| python-uv-pytest: | ||
| name: 🐍 Run Python tests with uv | ||
| needs: detect-repo-capabilities | ||
| if: needs.detect-repo-capabilities.outputs.has_python_uv == 'true' | ||
| runs-on: ${{ vars.REPO_RUNNER_LABELS && fromJSON(vars.REPO_RUNNER_LABELS) || 'ubuntu-24.04' }} | ||
| steps: | ||
| - name: 📥 Check out | ||
| uses: actions/checkout@v6 | ||
|
|
||
| - name: ⚙️ Setup uv | ||
| uses: astral-sh/setup-uv@v5 | ||
|
|
||
| - name: Run checks via uv | ||
| run: uv run --frozen python -m pytest | ||
|
|
||
| bazel-bzlmod-lock-check: | ||
| name: 🔒 Check Bazel Bzlmod lockfile | ||
| needs: detect-repo-capabilities | ||
| if: needs.detect-repo-capabilities.outputs.has_bazel_lock == 'true' | ||
| uses: ./.github/workflows/bzlmod-lock-check.yml | ||
|
|
||
| bazel-module-name-check: | ||
| name: 📝 Check Bazel module name | ||
| needs: detect-repo-capabilities | ||
| # The script to detect the module name is broken | ||
| if: needs.detect-repo-capabilities.outputs.has_bazel == 'true' && false | ||
| runs-on: ${{ vars.REPO_RUNNER_LABELS && fromJSON(vars.REPO_RUNNER_LABELS) || 'ubuntu-24.04' }} | ||
| steps: | ||
| - name: 📥 Check out | ||
| uses: actions/checkout@v6 | ||
|
|
||
| - name: 📝 Validate Bazel module name | ||
| run: | | ||
| if ! MODULE_LINE=$(grep '^module(' MODULE.bazel | head -1); then | ||
| echo "❌ No module declaration found in MODULE.bazel" | ||
| exit 1 | ||
| fi | ||
|
|
||
| MODULE_NAME=$(echo "$MODULE_LINE" | sed 's/^module([[:space:]]*name[[:space:]]*=[[:space:]]*"\([^"]*\)".*/\1/' | sed 's/^module([[:space:]]*"\([^"]*\)".*/\1/') | ||
|
|
||
| if [[ -z "$MODULE_NAME" || "$MODULE_NAME" == "$MODULE_LINE" ]]; then | ||
| MODULE_NAME=$(echo "$MODULE_LINE" | grep -o '"[^"]*"' | head -1 | tr -d '"') | ||
| fi | ||
|
|
||
| echo "Found module name: $MODULE_NAME" | ||
|
|
||
| if [[ ! "$MODULE_NAME" =~ ^score_[[:lower:]_]+$ ]]; then | ||
| echo "❌ Invalid module name: $MODULE_NAME" | ||
| echo "Module name must match the pattern: ^score_[[:lower:]_]+$" | ||
| echo " - Must start with 'score_'" | ||
| echo " - Must contain only lowercase letters and underscores after 'score_'" | ||
| echo "Examples of valid names: score_cli, score_compose, score_web_api" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "✅ Module name is valid: $MODULE_NAME" | ||
|
|
||
| detect-repo-bazel-targets: | ||
| name: Detect repo bazel targets | ||
| needs: detect-repo-capabilities | ||
| if: needs.detect-repo-capabilities.outputs.has_bazel == 'true' | ||
| runs-on: ${{ vars.REPO_RUNNER_LABELS && fromJSON(vars.REPO_RUNNER_LABELS) || 'ubuntu-24.04' }} | ||
| outputs: | ||
| has_copyright_check: ${{ steps.detect.outputs.has_copyright_check }} | ||
| has_format_check: ${{ steps.detect.outputs.has_format_check }} | ||
|
|
||
| permissions: | ||
| id-token: write | ||
| contents: read | ||
|
|
||
| steps: | ||
| - name: 📥 Check out | ||
| uses: actions/checkout@v6 | ||
|
|
||
| - name: 📥 Check out cicd-workflows | ||
| uses: dariocurr/checkout-called@v6 | ||
| with: | ||
| path: .cicd-workflows | ||
|
|
||
| - name: ⚙️ Setup Bazel with shared caching | ||
| uses: bazel-contrib/setup-bazel@0.18.0 | ||
| with: | ||
| disk-cache: false | ||
| repository-cache: false | ||
| bazelisk-cache: true | ||
| cache-save: false | ||
|
|
||
| - name: 🕵️ Detect Bazel targets | ||
| id: detect | ||
| run: ./.cicd-workflows/scripts/detect-bazel-targets.sh "${{ github.workspace }}" | ||
|
|
||
| bazel-format-check: | ||
| name: 🖌️ Run bazel //:format.check | ||
| needs: detect-repo-bazel-targets | ||
| if: needs.detect-repo-bazel-targets.outputs.has_format_check == 'true' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure if this is really helpful. I woul;d say it shall be manual workflow input defaulted to true so by default formating shall be enabled. NOw if you integrate it and do not have formatting in bazel job is just green without any indication
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know whether formatting will be handled by bazel. Probably not. Maybe only in C++ / rust repos, maybe not, maybe not even there. The workflow is designed to be drop-in-able, rather than to enforce certain things. |
||
| runs-on: ${{ vars.REPO_RUNNER_LABELS && fromJSON(vars.REPO_RUNNER_LABELS) || 'ubuntu-24.04' }} | ||
| steps: | ||
| - name: 📥 Check out | ||
| uses: actions/checkout@v6 | ||
|
|
||
| - name: ⚙️ Setup Bazel with shared caching | ||
| uses: bazel-contrib/setup-bazel@0.18.0 | ||
| with: | ||
| disk-cache: false | ||
| repository-cache: false | ||
| bazelisk-cache: true | ||
| cache-save: false | ||
|
|
||
| - name: 🖌️ Run formatting check | ||
| run: bazel test //:format.check | ||
|
|
||
| bazel-copyright-check: | ||
| name: © Run bazel //:copyright.check | ||
| needs: detect-repo-bazel-targets | ||
| if: needs.detect-repo-bazel-targets.outputs.has_copyright_check == 'true' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Copyright is already moved to pre-commit AFAIK. So technically copyright.check is already legacy behavior. |
||
| runs-on: ${{ vars.REPO_RUNNER_LABELS && fromJSON(vars.REPO_RUNNER_LABELS) || 'ubuntu-24.04' }} | ||
| steps: | ||
| - name: 📥 Check out | ||
| uses: actions/checkout@v6 | ||
|
|
||
| - name: ⚙️ Setup Bazel with shared caching | ||
| uses: bazel-contrib/setup-bazel@0.18.0 | ||
| with: | ||
| disk-cache: false | ||
| repository-cache: false | ||
| bazelisk-cache: true | ||
| cache-save: false | ||
|
|
||
| - name: © Run copyright check | ||
| run: bazel run //:copyright.check | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| # ******************************************************************************* | ||
| # Copyright (c) 2026 Contributors to the Eclipse Foundation | ||
| # | ||
| # See the NOTICE file(s) distributed with this work for additional | ||
| # information regarding copyright ownership. | ||
| # | ||
| # This program and the accompanying materials are made available under the | ||
| # terms of the Apache License Version 2.0 which is available at | ||
| # https://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # ******************************************************************************* | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| repo="${1:?Repository path is required}" | ||
|
|
||
| if ! [[ -f "$repo/MODULE.bazel" || -f "$repo/WORKSPACE" || -f "$repo/WORKSPACE.bazel" ]]; then | ||
| echo "ERROR: Not a Bazel workspace; you cannot run this script here." >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if ! command -v bazel >/dev/null 2>&1; then | ||
| echo "ERROR: bazel is not available on PATH; you need that to run this script." >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| if [[ -z "${GITHUB_OUTPUT:-}" ]]; then | ||
| echo "WARNING: This action is designed to be used in a GitHub Actions workflow. Running in 'local mode'." >&2 | ||
| GITHUB_OUTPUT=/dev/stdout | ||
| fi | ||
|
|
||
| echo "::group:: Detecting Bazel targets" | ||
|
|
||
| # Runs a boolean-style check function and writes to GITHUB_OUTPUT. | ||
| emit_output() { | ||
| local name="$1" | ||
| shift | ||
|
|
||
| if "$@"; then | ||
| echo "$name=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "$name=false" >> "$GITHUB_OUTPUT" | ||
| fi | ||
| } | ||
|
|
||
| # Returns success if the given Bazel label can be queried from the repo root. | ||
| has_bazel_target() { | ||
| local target="$1" | ||
|
|
||
| ( | ||
| cd "$repo" | ||
| bazel query "$target" >/dev/null 2>&1 | ||
| ) | ||
| } | ||
|
|
||
| emit_output has_copyright_check has_bazel_target "//:copyright.check" | ||
| emit_output has_format_check has_bazel_target "//:format.check" | ||
|
|
||
| echo "::endgroup::" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| # ******************************************************************************* | ||
| # Copyright (c) 2026 Contributors to the Eclipse Foundation | ||
| # | ||
| # See the NOTICE file(s) distributed with this work for additional | ||
| # information regarding copyright ownership. | ||
| # | ||
| # This program and the accompanying materials are made available under the | ||
| # terms of the Apache License Version 2.0 which is available at | ||
| # https://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # ******************************************************************************* | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| repo="${1:?Repository path is required}" | ||
| if [[ -z "${GITHUB_OUTPUT:-}" ]]; then | ||
| echo "WARNING: This action is designed to be used in a GitHub Actions workflow. Running in 'local mode'." >&2 | ||
| GITHUB_OUTPUT=/dev/stdout | ||
| fi | ||
|
|
||
| echo "::group:: Detecting repository capabilities" | ||
|
|
||
| # Runs a boolean-style check function and writes to GITHUB_OUTPUT. | ||
| emit_output() { | ||
| local name="$1" | ||
| shift | ||
|
|
||
| if "$@"; then | ||
| echo "$name=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "$name=false" >> "$GITHUB_OUTPUT" | ||
| fi | ||
| } | ||
|
|
||
| # Returns success if any provided file path exists. | ||
| any_file_exists() { | ||
| local path | ||
| for path in "$@"; do | ||
| [[ -f "$path" ]] && return 0 | ||
| done | ||
| return 1 | ||
| } | ||
|
|
||
| # Returns success only if every provided file path exists. | ||
| all_files_exist() { | ||
| local path | ||
| for path in "$@"; do | ||
| [[ -f "$path" ]] || return 1 | ||
| done | ||
| return 0 | ||
| } | ||
|
|
||
| emit_output has_bazel any_file_exists \ | ||
| "$repo/MODULE.bazel" \ | ||
| "$repo/WORKSPACE" \ | ||
| "$repo/WORKSPACE.bazel" | ||
| emit_output has_bazel_lock any_file_exists \ | ||
| "$repo/MODULE.bazel.lock" | ||
| emit_output has_devcontainer any_file_exists \ | ||
| "$repo/.devcontainer/devcontainer.json" \ | ||
| "$repo/devcontainer.json" | ||
| emit_output has_python_uv all_files_exist \ | ||
| "$repo/pyproject.toml" \ | ||
| "$repo/uv.lock" | ||
| emit_output has_precommit any_file_exists \ | ||
| "$repo/.pre-commit-config.yaml" | ||
|
|
||
| echo "::endgroup::" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if a job from
on-pr.ymlfails? I assume this one also fails.I am asking because here I coded extra logic to check for failed jobs: https://github.com/eclipse-score/module_template/pull/57/changes#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR42-R62
Now I wonder if I could have achieved the same by using yet another workflow file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Potentially yeah, the same can be achieved via another indirection. Let's try that after this PR is merged?!