Skip to content
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

fix(ci/reviewdog): Flag files with CRLF line endings #37445

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

OnkarRuikar
Copy link
Contributor

Summary

When there is a file with CRLF(Windows) line endings pushed in a PR, Prettier converts them to LF. The workflow fails to suggest this change as a review comment, and the workflow doesn't fail either.

Solution

The git diff command doesn't consider CRLF -> LF as a change. CRLF -> LF change is not there in the diff output. Without the diff reviewdog can not make a PR comment.
As a workaround, we can flag files with CRLF and ask contributors to convert CRLF to LF.

Testing

Testing has been done in OnkarRuikar#44 (comment) and corresponding workflow run is: https://github.com/OnkarRuikar/content/actions/runs/12534736823/job/34955862300?pr=44#step:12:63

🪵 In the following files make sure all the lines end with 'lf' character and not 'crlf' characters:
i/lf    w/crlf  attr/text=auto eol=lf 	files/en-us/web/test/index.md

/cc @yin1999

@OnkarRuikar OnkarRuikar requested a review from a team as a code owner December 30, 2024 06:38
@github-actions github-actions bot added system [PR only] Infrastructure and configuration for the project size/s [PR only] 6-50 LoC changed labels Dec 30, 2024
@OnkarRuikar OnkarRuikar marked this pull request as draft December 31, 2024 01:09
@OnkarRuikar OnkarRuikar marked this pull request as ready for review December 31, 2024 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/s [PR only] 6-50 LoC changed system [PR only] Infrastructure and configuration for the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant