Skip to content

ci(pr-check-lint_content): run reviewdog in separate workflow#43510

Merged
LeoMcA merged 18 commits intomainfrom
separate-reviewdog
Mar 27, 2026
Merged

ci(pr-check-lint_content): run reviewdog in separate workflow#43510
LeoMcA merged 18 commits intomainfrom
separate-reviewdog

Conversation

@caugner
Copy link
Copy Markdown
Collaborator

@caugner caugner commented Mar 20, 2026

Description

  • Extracts the reviewdog portion of the pr-check-lint_content workflow into a separate pr-reviewdog workflow (privileged, with status reporting), by passing diff and Markdownlint log as an artifact.
  • Makes the pr-check-lint_content workflow unprivileged, and simplifies it by removing now-obsolete safeguards.

Motivation

  • Eliminate risk of code injection.
  • Make it easier to simplify the lint workflow in the future.

Additional details

Tested on my personal fork:

Note: Reviewdog refuses to use github-pr-review reporter when run in a non-PR workflow (workflow_run), so we need to pretend we're not in GitHub CI by hiding GITHUB_ACTIONS from it.

Related issues and pull requests

Same as:

@caugner caugner requested a review from a team as a code owner March 20, 2026 22:28
@caugner caugner requested review from LeoMcA and removed request for a team March 20, 2026 22:28
@github-actions github-actions bot added system [PR only] Infrastructure and configuration for the project size/m [PR only] 51-500 LoC changed labels Mar 20, 2026
@caugner caugner marked this pull request as draft March 23, 2026 14:49
@caugner

This comment was marked as outdated.

@caugner caugner force-pushed the separate-reviewdog branch from c48292f to 82a3338 Compare March 26, 2026 13:24
@caugner caugner marked this pull request as ready for review March 26, 2026 13:37
Copy link
Copy Markdown
Member

@LeoMcA LeoMcA left a comment

Choose a reason for hiding this comment

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

A few questions, but generally looks good:

Comment on lines +24 to +34
- name: Identify PR
id: identify-pr
env:
BASE_REPO: ${{ github.repository }}
GITHUB_TOKEN: ${{ github.token }}
HEAD_REPO: ${{ github.event.workflow_run.head_repository.full_name }}
HEAD_SHA: ${{ github.event.workflow_run.head_sha }}
run: |
PR_NUMBER=$(gh api "repos/$HEAD_REPO/commits/$HEAD_SHA/pulls" \
--jq ".[] | select(.base.repo.full_name == \"$BASE_REPO\") | .number")
echo "number=$PR_NUMBER" >> $GITHUB_OUTPUT
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thought for later: I remember us using a similar step to this across a bunch of workflows, short of publishing our own actions, could we have a library of "actions" we can import across a range of workflows (as I'm sure there's other similar steps we need across workflows).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The problem with actions (to be used as a step) is that afaik you need one repository per action, hence why https://github.com/Mozilla-Actions/ exists.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not sure if that's required, you can host multiple reusable actions in the same repo, MozCloud have reusable actions in this repo for example: https://github.com/mozilla-it/deploy-actions

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

IIUC composite actions run as jobs, not as steps. This could run as a separate job, but it would add overhead and slow things down.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hm, looks like that repo indeed has actions that can run as a step. All actions in one repo would share the same tag, but that might be fine for our own actions.

@caugner caugner force-pushed the separate-reviewdog branch from 51d3145 to 5c97bbd Compare March 27, 2026 08:13
@caugner caugner force-pushed the separate-reviewdog branch from 5c97bbd to 89a4658 Compare March 27, 2026 08:15
@caugner caugner force-pushed the separate-reviewdog branch from 89a4658 to 3adb680 Compare March 27, 2026 08:31
@caugner caugner force-pushed the separate-reviewdog branch from c3808fd to 89261d9 Compare March 27, 2026 09:19
Copy link
Copy Markdown
Member

@LeoMcA LeoMcA left a comment

Choose a reason for hiding this comment

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

Very nice!

@LeoMcA LeoMcA merged commit 78e6ce7 into main Mar 27, 2026
12 checks passed
@LeoMcA LeoMcA deleted the separate-reviewdog branch March 27, 2026 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m [PR only] 51-500 LoC changed system [PR only] Infrastructure and configuration for the project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants