-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Path filters: include deleted files in changed-file detection (Fixes #26356) #26540
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
base: master
Are you sure you want to change the base?
Path filters: include deleted files in changed-file detection (Fixes #26356) #26540
Conversation
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.
Pull request overview
This PR addresses issue #26356 by adding an opt-in include-deleted parameter to the get-changed-files action, enabling the path-filters composite action to consider deleted files when making CI decisions. The implementation follows a backward-compatible approach where the parameter defaults to false, preserving existing behavior for all current consumers (markdown link verifier and merge conflict checker) while allowing the path-filters action to opt-in by setting it to true.
Key changes:
- Added
include-deletedboolean input parameter toget-changed-filesaction with default value offalse - Updated file filtering logic to include
removedstatus files when the parameter is enabled, consistently across bothpull_requestandpushevent handlers - Enabled the new parameter (
include-deleted: 'true') in thepath-filtersaction to detect when files are deleted in relevant directories
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/actions/infrastructure/get-changed-files/action.yml |
Added include-deleted input parameter and updated filtering logic to conditionally include removed files in both PR and push event handlers |
.github/actions/infrastructure/get-changed-files/README.md |
Updated documentation to describe the new parameter and clarify that deleted files are filtered out by default unless opted-in |
.github/actions/infrastructure/path-filters/action.yml |
Enabled include-deleted: 'true' to receive deleted files for path filtering decisions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| script: | | ||
| const eventTypes = '${{ inputs.event-types }}'.split(',').map(t => t.trim()); | ||
| const filter = '${{ inputs.filter }}'; | ||
| const includeDeleted = ('${{ inputs.include-deleted }}' || 'false').toLowerCase() === 'true'; |
Copilot
AI
Nov 29, 2025
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.
[nitpick] The expression ('${{ inputs.include-deleted }}' || 'false') will not work as intended when the input is an empty string. In JavaScript, an empty string '' is falsy, so ('' || 'false') evaluates to 'false'. However, when the input is explicitly provided as an empty string, this could lead to unexpected behavior.
Consider simplifying to:
const includeDeleted = '${{ inputs.include-deleted }}'.toLowerCase() === 'true';This works correctly because:
- When not provided or empty:
''.toLowerCase() === 'true'→false✓ - When 'false':
'false'.toLowerCase() === 'true'→false✓ - When 'true':
'true'.toLowerCase() === 'true'→true✓
The current code works, but the || 'false' is unnecessary since the default is already set in the action input definition.
| const includeDeleted = ('${{ inputs.include-deleted }}' || 'false').toLowerCase() === 'true'; | |
| const includeDeleted = '${{ inputs.include-deleted }}'.toLowerCase() === 'true'; |
| |------|-------------|----------|---------| | ||
| | `filter` | Optional filter pattern (e.g., `*.md` for markdown files, `.github/` for GitHub files) | No | `''` | | ||
| | `event-types` | Comma-separated list of event types to support (`pull_request`, `push`) | No | `pull_request` | | ||
| | `include-deleted` | Include deleted/removed files in the results (`true`/`false`) | No | `false` | |
Copilot
AI
Nov 29, 2025
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.
[nitpick] Consider adding a usage example in the "Usage" section demonstrating the include-deleted parameter. Since this is a new feature, showing how to use it would improve discoverability.
For example:
# Include deleted files in the results (useful for CI decisions)
- name: Get all changed files including deletions
id: changed-files
uses: "./.github/actions/infrastructure/get-changed-files"
with:
include-deleted: 'true'While the parameter is documented in the inputs table, a concrete usage example would clarify when and how to use this opt-in feature.
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
|
@tcolvin2006 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
Fixes #26356 — Adds a minimal opt-in option to the changed-files action so the path-filters composite will receive deleted files (allowing CI decisions to consider removals), while all other consumers (including the markdown link verifier) keep the existing behavior; this change is intentionally small and low-risk.