Skip to content

Run format check on all PRs#5857

Closed
Copilot wants to merge 4 commits intomainfrom
copilot/run-format-check-on-all-prs
Closed

Run format check on all PRs#5857
Copilot wants to merge 4 commits intomainfrom
copilot/run-format-check-on-all-prs

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 5, 2026 •
edited
Loading

  • Remove paths-ignore from workflow-level triggers so the workflow fires on all PRs
  • Add a changes detection job that uses the GitHub API to check whether non-doc files changed
  • format job runs unconditionally on every PR
  • All other jobs (lint, smoke, test-*, tsc) are conditional on changes.outputs.code == 'true'
  • Add generous comments to the shell script in the changes detection job

Copilot AI linked an issue Apr 5, 2026 that may be closed by this pull request
🛠️ Repo: Run format check on all PRs #5856
Open
5 tasks
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Apr 5, 2026 •
edited
Loading

CLA Not Signed

@mark-wiemer
run format check on all PRs by extracting it to a dedicated workflow
db8907d 
Move the format:check job from mocha.yml (which uses paths-ignore and
skips doc/config-only PRs) into its own format.yml workflow with no
path filters so it runs on every PR.

Agent-Logs-Url: https://github.com/mochajs/mocha/sessions/b8339218-ca89-4d4f-bbfd-f4ef0beeb362

Co-authored-by: mark-wiemer <7833360+mark-wiemer@users.noreply.github.com>
Copilot AI changed the title [WIP] Add format check for all PRs Run format check on all PRs Apr 5, 2026
Copilot AI requested a review from mark-wiemer April 5, 2026 01:33
@mark-wiemer
Copy link
Copy Markdown
Member

mark-wiemer commented Apr 5, 2026

@copilot Instead of creating a new workflow file, can we apply the paths-ignore property at a job-by-job level?

@mark-wiemer
apply paths-ignore at job level instead of separate workflow
ca5811f 
Remove paths-ignore from workflow-level triggers and add a changes
detection job that uses the GitHub API to check if non-doc files
changed. The format job always runs; all other jobs are conditional
on changes.outputs.code to preserve the original skip behavior for
documentation-only PRs.

Agent-Logs-Url: https://github.com/mochajs/mocha/sessions/8bab526a-e11d-479f-9ce4-2f4f7b4f1e32

Co-authored-by: mark-wiemer <7833360+mark-wiemer@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 5, 2026

@copilot Instead of creating a new workflow file, can we apply the paths-ignore property at a job-by-job level?

Done in ca5811f. Instead of a separate file, I moved the path filtering to the job level within mocha.yml:

  • Removed paths-ignore from the workflow-level triggers so the workflow fires on all PRs
  • Added a changes job that uses the GitHub API to detect whether non-doc files changed
  • format has no conditions and always runs
  • All other jobs (lint, smoke, test-*, tsc) are conditional on needs.changes.outputs.code == 'true', preserving the original skip behavior for documentation-only PRs

@mark-wiemer
Copy link
Copy Markdown
Member

mark-wiemer commented Apr 5, 2026 •
edited
Loading

@copilot we humans are bad at reading shell scripts, please add generous comments to it. (Code LGTM rn, want to make sure I'm understanding the shell stuff correctly, might as well document it for next time)

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 5, 2026

@copilot we humans are bad at reading shell scripts, please add generous comments to it

Added comments throughout the shell script in 95f61ac. Each block now explains what it does and why.

@mark-wiemer mark-wiemer marked this pull request as ready for review April 5, 2026 21:25
mark-wiemer
mark-wiemer approved these changes Apr 5, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@mark-wiemer mark-wiemer left a comment

Choose a reason for hiding this comment

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

#ripjohnhenry

@mark-wiemer mark-wiemer requested a review from a team April 5, 2026 21:26
@mark-wiemer
Copy link
Copy Markdown
Member

mark-wiemer commented Apr 5, 2026

Will merge in 1-7 calendar days while I wait for another maintainer to review if they're curious :)

JoshuaKGoldberg
JoshuaKGoldberg reviewed Apr 5, 2026
View reviewed changes
.github/workflows/mocha.yml

jobs:
# Detect whether non-doc files changed so test jobs can be skipped on
# documentation-only PRs while format:check still runs on every PR.
Copy link
Copy Markdown
Member

@JoshuaKGoldberg JoshuaKGoldberg Apr 5, 2026

Choose a reason for hiding this comment

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

This is some fancy scripting. I'd rather use something like https://github.com/marketplace/actions/changed-files / https://github.com/RasaHQ/pr-changed-files-filter / similar. WDYT?

@voxpelli
Copy link
Copy Markdown
Member

voxpelli commented Apr 6, 2026

Do we really need to use an AI to make this change? 🙈

It seems to be inventing a lot of needless stuff here and the challenge with AI generated stuff is that it increases the burden on reviewers.

@mark-wiemer
Copy link
Copy Markdown
Member

mark-wiemer commented Apr 6, 2026

I figured it was worth a shot, we can abandon this approach

@mark-wiemer mark-wiemer closed this Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JoshuaKGoldberg JoshuaKGoldberg JoshuaKGoldberg left review comments

+1 more reviewer

@mark-wiemer mark-wiemer mark-wiemer approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mark-wiemer mark-wiemer

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

🛠️ Repo: Run format check on all PRs

4 participants

@mark-wiemer @voxpelli @JoshuaKGoldberg