Skip to content

Comments

CQ: Replace crlf check with validation against an .editorconfig file#6070

Merged
echoix merged 26 commits intoOSGeo:mainfrom
echoix:editorconfig-crlf
Jul 17, 2025
Merged

CQ: Replace crlf check with validation against an .editorconfig file#6070
echoix merged 26 commits intoOSGeo:mainfrom
echoix:editorconfig-crlf

Conversation

@echoix
Copy link
Member

@echoix echoix commented Jul 16, 2025

Simplified and adapted from #4792.
Fixes #6066 (comment)

The action that checks for CRLF has been failing since at least this weekend. I took a look this weekend, and since the action builds a docker image on the fly each time, we are affected if any of the services called during the image built are down.
Here, it was using an older Debian 10 (buster), and installed additional software. It seems that the package repository URLs are not available at the same location as before. Probably not removed, but some manual tweeking must be done to set the new URL. I couldn’t find an announcement or hints on when the change have been made. I didn’t see much other projects doing some changes to the install steps, but still found some.

Considering that that action has low chances of beeing fixed in a timely matter, I replaced the functionality with an .editorconfig file and editorconfig-checker. Compared to #4792, here I don’t attempt to make the line ending changes persist in the repo through .gitattributes, and ignore a bit more of non-ready config sections (since the functionality replaced was less detailed than what the .editorconfig I already had almost working).

I made the pre-commit run always, and made it faster in consequence to not add a big impact. With hot cache, it is taking 1min-1min10, which is about the same. I knew pre-commit could be faster in CI as I already saw how fast it was in ruff’s own CI when submitting one of my PRs there once. It can still be a bit faster, but it’s enough for today.

@echoix echoix requested a review from petrasovaa July 16, 2025 01:33
@github-actions github-actions bot added the CI Continuous integration label Jul 16, 2025
@echoix
Copy link
Member Author

echoix commented Jul 16, 2025

I just didn’t have the chance to try this in a Windows environment. But since there’s no actual fixing, or .gitattributes involved, there’s no reason to have problems like I already witnessed myself in previous work.

Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

Great addition. Just a minor change needed.

@echoix echoix requested a review from wenzeslaus July 16, 2025 11:09
@echoix echoix merged commit cf2ad99 into OSGeo:main Jul 17, 2025
28 checks passed
@github-actions github-actions bot added this to the 8.5.0 milestone Jul 17, 2025
@petrasovaa
Copy link
Contributor

Addons have the same issue

@echoix
Copy link
Member Author

echoix commented Jul 18, 2025

Addons have the same issue

I might not have time before late this weekend. Can someone use what I did, skipping most unneeded sections of the .editorconfig file? (I don't think there's many exceptions to keep)

@echoix
Copy link
Member Author

echoix commented Jul 18, 2025

At least disable the crlf check, we'd rather have this missing than none of the checks working at all

echoix added a commit to echoix/grass that referenced this pull request Sep 21, 2025
…SGeo#6070)

Simplified and adapted from  OSGeo#4792.
Fixes OSGeo#6066 (comment)

The action that checks for CRLF has been failing since at least this weekend.
Considering that that action has low chances of beeing fixed in a timely matter, I replaced the functionality with an .editorconfig file and validating against it with editorconfig-checker. Compared to OSGeo#4792, here I don’t attempt to make the line ending changes persist in the repo through .gitattributes, and ignore a bit more of non-ready config sections (since the functionality replaced was less detailed than what the .editorconfig I already had almost working).

I made the pre-commit run always, and made it faster in consequence to not add a big impact. With hot cache, it is taking 1min-1min10, which is about the same. I knew pre-commit could be faster in CI as I already saw how fast it was in ruff’s own CI when submitting one of my PRs there once. It can still be a bit faster, but it’s enough for today.

* CQ: Add initial .editorconfig file with basic settings

* CI: Add EditorConfig validation to super-linter workflow

* CI: Remove CRLF check from additional_checks.yml workflow

* CQ: Add editorconfig-checker to pre-commit config

* CI: Always run pre-commit in CI

* Update .editorconfig with new whitespace settings for mswindows

* CI: Run pre-commit step with uvx

* CI: Add caching for pre-commit checks in workflow

* Update .editorconfig for Windows batch files

* CI: Remove paths-filter step from additional_checks.yml

* Update additional_checks.yml step setup-uv
echoix added a commit to echoix/grass that referenced this pull request Sep 25, 2025
…SGeo#6070)

Simplified and adapted from  OSGeo#4792.
Fixes OSGeo#6066 (comment)

The action that checks for CRLF has been failing since at least this weekend.
Considering that that action has low chances of beeing fixed in a timely matter, I replaced the functionality with an .editorconfig file and validating against it with editorconfig-checker. Compared to OSGeo#4792, here I don’t attempt to make the line ending changes persist in the repo through .gitattributes, and ignore a bit more of non-ready config sections (since the functionality replaced was less detailed than what the .editorconfig I already had almost working).

I made the pre-commit run always, and made it faster in consequence to not add a big impact. With hot cache, it is taking 1min-1min10, which is about the same. I knew pre-commit could be faster in CI as I already saw how fast it was in ruff’s own CI when submitting one of my PRs there once. It can still be a bit faster, but it’s enough for today.

* CQ: Add initial .editorconfig file with basic settings

* CI: Add EditorConfig validation to super-linter workflow

* CI: Remove CRLF check from additional_checks.yml workflow

* CQ: Add editorconfig-checker to pre-commit config

* CI: Always run pre-commit in CI

* Update .editorconfig with new whitespace settings for mswindows

* CI: Run pre-commit step with uvx

* CI: Add caching for pre-commit checks in workflow

* Update .editorconfig for Windows batch files

* CI: Remove paths-filter step from additional_checks.yml

* Update additional_checks.yml step setup-uv
echoix added a commit that referenced this pull request Sep 25, 2025
…6070)

Simplified and adapted from  #4792.
Fixes #6066 (comment)

The action that checks for CRLF has been failing since at least this weekend.
Considering that that action has low chances of beeing fixed in a timely matter, I replaced the functionality with an .editorconfig file and validating against it with editorconfig-checker. Compared to #4792, here I don’t attempt to make the line ending changes persist in the repo through .gitattributes, and ignore a bit more of non-ready config sections (since the functionality replaced was less detailed than what the .editorconfig I already had almost working).

I made the pre-commit run always, and made it faster in consequence to not add a big impact. With hot cache, it is taking 1min-1min10, which is about the same. I knew pre-commit could be faster in CI as I already saw how fast it was in ruff’s own CI when submitting one of my PRs there once. It can still be a bit faster, but it’s enough for today.

* CQ: Add initial .editorconfig file with basic settings

* CI: Add EditorConfig validation to super-linter workflow

* CI: Remove CRLF check from additional_checks.yml workflow

* CQ: Add editorconfig-checker to pre-commit config

* CI: Always run pre-commit in CI

* Update .editorconfig with new whitespace settings for mswindows

* CI: Run pre-commit step with uvx

* CI: Add caching for pre-commit checks in workflow

* Update .editorconfig for Windows batch files

* CI: Remove paths-filter step from additional_checks.yml

* Update additional_checks.yml step setup-uv
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants