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

pr-check-lint_content workflow will not failed when there are only EOL changes #25244

Open
yin1999 opened this issue Dec 28, 2024 · 6 comments
Open
Labels
system Infrastructure and configuration for the project

Comments

@yin1999
Copy link
Member

yin1999 commented Dec 28, 2024

I found the file (files/zh-cn/web/api/uievent/sourcecapabilities/index.md) in #25207 is using the crlf EOLs which will be replaced with lf by prettier, see the workflow run: https://github.com/mdn/translated-content/actions/runs/12510390631/job/34901079183#step:10:62

image

We need a solution to detect this error in PR.


Addition: the clean-up job is now handled by the daily markdownlint auto-cleanup (the cleanup PR for the above file: #25242)

@yin1999 yin1999 added the system Infrastructure and configuration for the project label Dec 28, 2024
@yin1999
Copy link
Member Author

yin1999 commented Dec 28, 2024

/cc @OnkarRuikar. Do you have any idea about this. Maybe the mdn/content repository also has this issue, is there any discussion on this :)

@OnkarRuikar
Copy link
Contributor

OnkarRuikar commented Dec 28, 2024

😕 I couldn't see crlf endings in the file both in the PR commits and on the main branch after merge, it's only lfs. Let me know how what tools you used to see `crlfs. Many editors auto-correct line endings, so it's very confusing.

I think it's the missing lf at the end of the last line. Please confirm.

@yin1999
Copy link
Member Author

yin1999 commented Dec 28, 2024

😕 I couldn't see crlf endings in the file both in the PR commits and on the main branch after merge, it's only lfs. Let me know how what tools you used to see `crlfs. Many editors auto-correct line endings, so it's very confusing.

Hey @OnkarRuikar, if you download the raw file corresponding to the commit (https://github.com/mdn/translated-content/blob/8eebac6a71750a410824ccda0919e9083e60ea3f/files/zh-cn/web/api/uievent/sourcecapabilities/index.md) directly, you would be able to see the crlf EOL. As markdownlint auto-cleanup has created a PR to fix the EOL early today.

@OnkarRuikar
Copy link
Contributor

I've made the required changes to the workflow but cannot test it using my Linux environment. I can make a commit with crlf file, but when I push to GitHub it's automatically converted to lf. GitHub editor is not helpful either.

I don't understand how they managed to get the file with CRLF committed with the current settings. Let me know how to get a file with crlf in a PR. I've given access to you to my repo. Could you push a file with crlf to this PR: OnkarRuikar/content#44 ? Or test it in your local?

@yin1999
Copy link
Member Author

yin1999 commented Dec 29, 2024

I've made the required changes to the workflow but cannot test it using my Linux environment. I can make a commit with crlf file, but when I push to GitHub it's automatically converted to lf. GitHub editor is not helpful either.

I don't understand how they managed to get the file with CRLF committed with the current settings. Let me know how to get a file with crlf in a PR. I've given access to you to my repo. Could you push a file with crlf to this PR: OnkarRuikar/content#44 ? Or test it in your local?

I can't reproduce this, let me get some information from the PR's author ^_^

@yin1999
Copy link
Member Author

yin1999 commented Dec 29, 2024

He used github.dev to submit the changes. If we manually convert the EOL to CRLF in the web UI, the EOL only changes can be committed to the remote repository.


Commit in your fork using github.dev: OnkarRuikar/content@9490931

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
system Infrastructure and configuration for the project
Projects
None yet
Development

No branches or pull requests

2 participants