Fix style issues from editorconfig configuration

[hiddewie]

Ref #4965 (comment)

Indicated by @pnorman to be put into a separate pull request.

This pull request fixes formatting issues compared to the configured editorconfig.

[hiddewie]

Looks better now. The linter found more issues with the scripts / docs / code, which I fixed. I disabled the rule to check for indent sizes matching the editorconfig, because there were too many errors that did not seem like big issues (indenting in SQL queries for example).

[imagico]

I am fine with removing end-of-line whitespace from files and making formatting more consistent - although it is worth to consider doing this after we finish changes currently in the works with more extensive changes to the code (#4952, #4965).

I am not ok with elevating a file that has been introduced as an optional helper for developers to binding policy without discussion.

I am generally wary about CI tools generating cryptic error messages without guidance how to fix or documentation on the underlying principles. This is, unfortunately, common in open source projects meanwhile and de facto (intentionally or not) discourages/excludes less technically minded people from contributing. We should not do that. If we want CI to enforce cosmetic coding requirements it should generate clearly worded messages about (a) what is wrong and (b) how to fix it.

[hiddewie]

I am generally wary about CI tools generating cryptic error messages without guidance how to fix or documentation on the underlying principles.

For me the messages in https://github.com/gravitystorm/openstreetmap-carto/actions/runs/9042451722/job/24848982515#step:17:54 are pretty clear, annotated with the file, the line number and the problem.

This is, unfortunately, common in open source projects meanwhile and de facto (intentionally or not) discourages/excludes less technically minded people from contributing. We should not do that.

To be honest I prefer CI to tell me what is wrong with my PR instead of relying on a human review to verify and correct any cosmetic style changes. Especially for open source projects where maintainer time is scarce, and that time is better spent on validating things only humans can validate in a pull request.

I am not ok with elevating a file that has been introduced as an optional helper for developers to binding policy without discussion.

OK, I will remove all enforcement of the style changes from this PR. The maintainers can have a discussion about introducing such checks in CI.

[imagico]

I am generally wary about CI tools generating cryptic error messages without guidance how to fix or documentation on the underlying principles.

For me the messages in https://github.com/gravitystorm/openstreetmap-carto/actions/runs/9042451722/job/24848982515#step:17:54 are pretty clear, annotated with the file, the line number and the problem.

I admit i have seen much worse in that regard. But still this is not exactly inviting for contributors, especially not if a minor formatting issue in a markdown file is shown as an error just like a hard coding error preventing the style from working.

Also curious that you need to allow external content from windows.net to get the detailed information (wtf?).

This is, unfortunately, common in open source projects meanwhile and de facto (intentionally or not) discourages/excludes less technically minded people from contributing. We should not do that.

To be honest I prefer CI to tell me what is wrong with my PR instead of relying on a human review to verify and correct any cosmetic style changes. Especially for open source projects where maintainer time is scarce, and that time is better spent on validating things only humans can validate in a pull request.

To be clear - i am not at all against using automated tools to help developers improve their PRs. I am against deploying those tools as a gatekeeper discouraging contributions by indicating them to be faulty possibly just because some indention inconsistency in a documentation file. If we can reduce the formatting things to a warning or info level instead of an error i would be happy including that.

Personally i think formal whitespace issues are not a very important aspect and it would be much more valuable to establish some more regular code maintenance works that looks for higher level inconsistencies (which are not caught by the tools discussed here).

[hiddewie]

True.

This PR is mostly meant as a time saver for both people making PRs and reviewing them: making PRs is difficult because editors that honour the .editorconfig file will make changes to files (like strip whitespace, and end a file with a newline). These changes should not be committed in PRs that are not about that topic. Locally this means the contributor has changes that should not be committed. And if the changes are committed by accident, reviewers get busy reviewing unintended changes that should not been committed (e.g. #4965 (comment)).

All that time is better spent on changes that matter instead of discussing whitespace.