Skip to content

Conversation

@colleenmcginnis
Copy link
Contributor

In elastic/docs-content#3756 we're removing two Markdown files. When the Vale check runs, it errors out because issues is not iterable.

I think this happens because when it tries to find the lines that were changed in the deleted files, it results in an error that messes up the content in the vale_output.json file. I added --diff-filter=d to git diff to filter out deleted files when getting a list of files that have changed and need to be linted.

I tested this locally and it seems to work, but I'm open to other suggestions!

Copy link
Collaborator

@jmikell821 jmikell821 left a comment

Choose a reason for hiding this comment

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

On my end, this LGTM, but will wait for a double confirmation from Fabri. Thanks for opening! ❤️

@theletterf theletterf requested a review from Copilot November 22, 2025 08:46
Copy link

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 fixes an error in the Vale linting workflow that occurs when files are deleted in a pull request. The issue manifests as an unhandled error when attempting to lint non-existent files, causing the issues variable to become non-iterable.

Key Changes:

  • Added --diff-filter=d flag to git diff commands to exclude deleted files from linting
  • Applied the fix to both the file discovery step and the line range detection step

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@theletterf
Copy link
Collaborator

Thanks for the catch, @colleenmcginnis! Very elegant fix, too! Applying this ASAP.

@theletterf theletterf merged commit 25c2ce6 into main Nov 22, 2025
1 check passed
@colleenmcginnis colleenmcginnis deleted the cmcg-fix-error branch November 22, 2025 13:39
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.

4 participants