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

[INFRA] add CI to find trailing whitespace #780

Merged
merged 6 commits into from
Apr 29, 2021

Conversation

sappelhoff
Copy link
Member

This PR adds a CI steps that alerts us about trailing whitespace.

With some users having their editors configured to automatically remove it and others not, we are in a situation where we continuously add and remove senseless trailing whitespace. That negatively affects the git diff and makes review harder.

This CI step should solve the problem by alerting us to not merge any changes with trailing whitespace.

Came up as part of #777

Copy link
Member Author

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

Okay, this also looks at binary files. It seems like the GitHub action I found it not very far in its development and may lack some features.

I'll return to this PR in the near future. If any of you have suggestions and/or comments meanwhile --> please let me know.

@tsalo
Copy link
Member

tsalo commented Apr 19, 2021

I'm a little surprised that the Markdown linter doesn't catch trailing whitespace. Is that not an option for that Action?

@sappelhoff
Copy link
Member Author

Thanks @tsalo I did not think of that. It's a much better solution and it seems to work. Please review if you can find some time :-)

Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

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

LGTM!

@sappelhoff sappelhoff merged commit 03c625b into bids-standard:master Apr 29, 2021
@sappelhoff sappelhoff deleted the ci/whitespace branch April 29, 2021 07:19
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