Workflow standardization and improvements#290
Conversation
There was a problem hiding this comment.
Pull request overview
This PR standardizes and improves GitHub Actions workflows across the repository, focusing on consistency, correctness, and maintainability. The changes consolidate patterns for workflow triggers, input/output handling, and shell script compatibility while adding new functionality such as Python coverage monitoring and enhanced commit result outputs.
Changes:
- Standardized pre-check job outputs (
ref,repo,base_sha) across all check workflows for consistent checkout operations - Improved shell compatibility by replacing bash-specific syntax (
[[) with POSIX-compliant syntax ([) - Enhanced security by reordering trap/echo statements and standardizing author association checks
- Added Python test coverage monitoring to the coverage workflow
- Fixed actionlint issue in jsonnet-format-fix by replacing job-level container with docker run command
- Updated download-artifact action to v7.0.0 with correct hash
- Added commit result outputs to handle-fix-commit action (commit_sha_short, etc.)
- Improved result evaluation logic with separate steps and proper skip conditions
- Standardized bot command naming (e.g.,
jsonnet-fixinstead ofjsonnet-format-fix) - Updated workflow run-name descriptions for better clarity
- Comprehensively updated documentation to reflect all workflow changes
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/python-check.yaml |
Added ref/repo/base_sha outputs, split ruff/mypy checks, improved result evaluation |
.github/workflows/python-fix.yaml |
Updated description, added MEMBER to author check, standardized tool name to "ruff" |
.github/workflows/markdown-check.yaml |
Added ref/repo/base_sha outputs, standardized conditions, improved run-name |
.github/workflows/markdown-fix.yaml |
Improved run-name, added MEMBER to author check, removed legacy @phlexbot commands |
.github/workflows/jsonnet-format-check.yaml |
Added base_sha output, standardized conditions and evaluation logic |
.github/workflows/jsonnet-format-fix.yaml |
Fixed actionlint issue by using docker run, renamed command to jsonnet-fix |
.github/workflows/cmake-format-check.yaml |
Added ref/repo/base_sha outputs, split check/evaluation, improved conditions |
.github/workflows/cmake-format-fix.yaml |
Improved run-name consistency, added MEMBER to auth, renamed command to cmake-fix |
.github/workflows/cmake-build.yaml |
Refactored to use get-pr-info action, added MEMBER to auth, improved conditions |
.github/workflows/clang-format-check.yaml |
Added ref/repo outputs, improved evaluation with skipped check, shell compatibility |
.github/workflows/clang-format-fix.yaml |
Improved run-name, added MEMBER to auth, pinned action to @main, renamed to clang-fix |
.github/workflows/clang-tidy-check.yaml |
Added pre-check job, ref/repo/base_sha outputs, skipped job, improved conditions |
.github/workflows/clang-tidy-fix.yaml |
Improved run-name, added MEMBER to auth, pinned action to @main, fixed bot name handling |
.github/workflows/actionlint-check.yaml |
Added ref/repo/base_sha outputs, improved evaluation and conditions, shell compatibility |
.github/workflows/coverage.yaml |
Added Python to coverage file types, added ref/repo/base_sha outputs |
.github/workflows/codeql-analysis.yaml |
Added set -eu for safer shell scripts |
.github/workflows/codeql-comment.yaml |
Updated download-artifact to v7.0.0 with correct hash |
.github/actions/handle-fix-commit/action.yaml |
Added outputs (commit_sha_short, etc.), reordered trap/echo for security |
.github/actions/get-pr-info/action.yaml |
Added base_sha output |
.github/REUSABLE_WORKFLOWS.md |
Updated all examples to reflect workflow changes, corrected command names |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (74.60%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. @@ Coverage Diff @@
## main #290 +/- ##
==========================================
+ Coverage 74.28% 74.60% +0.32%
==========================================
Files 124 124
Lines 2955 2961 +6
Branches 513 516 +3
==========================================
+ Hits 2195 2209 +14
+ Misses 540 532 -8
Partials 220 220
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
@phlexbot format |
|
No automatic cmake-format fixes were necessary. |
|
No automatic clang-format fixes were necessary. |
7cb3c21 to
dfb1a60
Compare
|
@phlexbot format |
|
No automatic cmake-format fixes were necessary. |
|
No automatic clang-format fixes were necessary. |
knoepfel
left a comment
There was a problem hiding this comment.
Bravo, @greenc-FNAL. Just a couple questions
- Update `jsonnet-format-fix.yaml`** to use environment variables and
`${{ github.workspace }}` for better compliance and safety in `docker
run`.
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
- Improve/standardize `name`, `run-name`, `description` - Add `ref`, `repo`, `base_sha` where appropriate - Improve and standardize prerequisite conditions - Standardize authorization checks for "fix" workflows - Standardize comment invocation for "fix workflows - e.g. `@phlexbot clang-fix` (✓) _vs_ `@phlexbot clang-format-fix` (❌) - Use the actual tool name: - `jsonnetfmt` (✓) _vs_ `jsonnet-format` (❌) - `ruff` (✓) _vs_ "Python linting" (❌)
This change addresses the issue where `cmake-build` and other workflows were detecting irrelevant changes when a PR branch was not fully rebased or when the base branch had moved forward. Key improvements: - Updated `detect-relevant-changes` action to use triple-dot diff (`git diff BASE...HEAD`), ensuring only changes introduced in the feature branch are considered. - Standardized `pre-check` job outputs across all relevant workflows (`cmake-build.yaml`, `clang-format-check.yaml`, `clang-tidy-check.yaml`, `coverage.yaml`, etc.) for consistent ref, repo, and base_sha handling. - Improved `base_sha` fallback logic to robustly handle various trigger types (pull_request, push, issue_comment, workflow_call). - Renamed `sha` output to `ref` in `cmake-build.yaml` to match other workflows and updated downstream usages. - Added `workflow_call` support to `clang-format-check.yaml` and `clang-tidy-check.yaml` to match the standardized pattern. Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
* Initial plan * Add authorization analysis documentation Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com> * Add inline authorization comments to workflows Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com> * Complete authorization review and documentation Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com> * Remove accidentally committed actionlint binary Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com> * Initial analysis of actionlint and injection issues Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com> * Fix actionlint errors and injection vulnerability in workflows Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com> * Add documentation for actionlint and injection fixes Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com> * Remove incorrect workflow_call support from check workflows Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
6ccbc9b to
0e56057
Compare
actionlintissue withjsonnet-format-fix.yamldownload-artifactbase_shaas an outputsh@main