Skip to content

Enable fork-based PRs to use fix workflows#157

Merged
knoepfel merged 7 commits intomainfrom
jules-maintenance-enable-fork-workflows
Dec 5, 2025
Merged

Enable fork-based PRs to use fix workflows#157
knoepfel merged 7 commits intomainfrom
jules-maintenance-enable-fork-workflows

Conversation

@greenc-FNAL
Copy link
Copy Markdown
Contributor

This commit refactors the "fix" workflows (clang-tidy, clang-format, cmake-format, and python-fix) to allow contributors from forked repositories to trigger them.

Key changes:

  • A new reusable action, handle-fix-commit, is introduced to encapsulate the logic for either committing fixes directly or creating a patch file.
  • The workflows now check if the PR author has enabled "allow edits from maintainers." If so, the fixes are committed to their branch.
  • If edits are not allowed, a patch file is created and uploaded as an artifact, and a comment is posted on the PR with instructions on how to apply it.
  • The author_association check has been removed from the workflows, relying on GitHub's "require approval for first-time contributors" setting for security.
  • The python-fix.yaml workflow has been corrected to include the necessary pull-requests: write permission.

@greenc-FNAL greenc-FNAL marked this pull request as ready for review December 4, 2025 22:05
Copilot AI review requested due to automatic review settings December 4, 2025 22:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the fix workflows (clang-tidy, clang-format, cmake-format, and python-fix) to enable fork-based pull requests to use automated fix functionality. Previously, the workflows were restricted to collaborators and owners via author_association checks. Now, the workflows rely on GitHub's "require approval for first-time contributors" setting for security and can either commit fixes directly (if the PR author has enabled "allow edits from maintainers") or provide a patch file for manual application.

Key changes:

  • Introduces a reusable composite action handle-fix-commit that encapsulates the commit/patch logic
  • Removes author_association barriers from all fix workflow triggers
  • Adds logic to check the maintainer_can_modify property on PRs to determine if direct commits are possible
  • Implements a fallback mechanism that creates patch files and posts instructions when direct commits aren't allowed

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
.github/actions/handle-fix-commit/action.yaml New reusable composite action that checks for changes, attempts to commit if possible (based on fork status and maintainer edit permissions), or creates a patch artifact with instructions
.github/workflows/python-fix.yaml Refactored to use new handle-fix-commit action; removed author_association check; restructured with separate parse-command and apply_fixes jobs
.github/workflows/cmake-format-fix.yaml Refactored to use new handle-fix-commit action; removed author_association check; simplified commit handling
.github/workflows/clang-tidy-fix.yaml Refactored to use new handle-fix-commit action; removed author_association check; streamlined fix application process
.github/workflows/clang-format-fix.yaml Refactored to use new handle-fix-commit action; removed author_association check; consolidated commit logic
Comments suppressed due to low confidence (2)

.github/workflows/python-fix.yaml:39

  • The apply_fixes job has a redundant Get PR Info step (lines 37-39) that is not used. The job already receives the PR information from the parse-command job's outputs (lines 44-45 use needs.parse-command.outputs.*). This duplicate step should be removed.
      - name: Get PR Info
        id: get_pr
        uses: Framework-R-D/phlex/.github/actions/get-pr-info@main

.github/workflows/python-fix.yaml:39

  • In the python-fix.yaml workflow, the apply_fixes job still has a duplicate Get PR Info step (lines 37-39) even though it receives the PR info from the parse-command job's outputs. This step is redundant and should be removed since the outputs from parse-command are already being used (lines 44-45).
      - name: Get PR Info
        id: get_pr
        uses: Framework-R-D/phlex/.github/actions/get-pr-info@main

This commit refactors the "fix" workflows (`clang-tidy`, `clang-format`, `cmake-format`, and `python-fix`) to allow contributors from forked repositories to trigger them.

Key changes:
- A new reusable action, `handle-fix-commit`, is introduced to encapsulate the logic for either committing fixes directly or creating a patch file.
- The workflows now check if the PR author has enabled "allow edits from maintainers." If so, the fixes are committed to their branch.
- If edits are not allowed, a patch file is created and uploaded as an artifact, and a comment is posted on the PR with instructions on how to apply it.
- The `author_association` check has been removed from the workflows, relying on GitHub's "require approval for first-time contributors" setting for security.
- The `python-fix.yaml` workflow has been corrected to include the necessary `pull-requests: write` permission.
@greenc-FNAL greenc-FNAL force-pushed the jules-maintenance-enable-fork-workflows branch from c46ddb6 to ed3c0a3 Compare December 4, 2025 22:09
greenc-FNAL and others added 6 commits December 4, 2025 16:12
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@knoepfel knoepfel merged commit fbd7b3e into main Dec 5, 2025
34 checks passed
@knoepfel knoepfel deleted the jules-maintenance-enable-fork-workflows branch December 5, 2025 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants