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

Add .editorconfig and CI check for trailing whitespace #876

Merged
merged 1 commit into from
Jul 28, 2024

Conversation

DeinAlptraum
Copy link
Contributor

@DeinAlptraum DeinAlptraum commented Jul 26, 2024

The editorconfig should prevent trailing whitespaces and ensure a trailing newline in all files.
The CI adds another job that runs on PRs only, marking all spots in src (though ignoring src/lib, src/webSDK) changed in that revision that added trailing spaces. An example of the output in case of failure can be seen here.

This encompasses stage 1 & 2 from #873

Open question: Should the .editorconfig apply to all files, or should we limit this to the same folders as the CI check?

@barbeque-squared
Copy link
Member

Open question: Should the .editorconfig apply to all files, or should we limit this to the same folders as the CI check?

Imo there's no point in spending time figuring out what would be the "perfect" editorconfig because if it turns out it needs changing, that's super easy. It's better than nothing at all and over time it'll just converge towards whatever "perfect" turns out to be.
Compare it to adding dozens of ultra exotic Lua functions: what we choose now will have effects years into the future. Editorconfig only applies until the next time it's changed.

Copy link
Member

@barbeque-squared barbeque-squared left a comment

Choose a reason for hiding this comment

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

Looks good!

@barbeque-squared barbeque-squared merged commit e3c1b81 into UltraStar-Deluxe:master Jul 28, 2024
5 checks passed
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.

2 participants