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

Make Trailing Whitespace Tests Optional #745

Closed
zoffixznet opened this issue Jul 22, 2016 · 6 comments
Closed

Make Trailing Whitespace Tests Optional #745

zoffixznet opened this issue Jul 22, 2016 · 6 comments
Assignees

Comments

@zoffixznet
Copy link
Contributor

As per IRC conversation, opening this ticket to suggest making trailing whitespace tests optional.

The reasoning is they often break builds and require contributors to spend time unbreaking them, even though the presence of an occasional trailing whitespaces isn't as detrimental to justify paying such a price.

In an ideal world, there'd be some block that'd prevent a commit from entering if it has trailing whitespace. In a less ideal world, there'd be an automated service unbreaking commits with such breakage. And the last option is to disable the test altogether and learn to live with trailing whitespace.

@coke coke self-assigned this Jul 22, 2016
@AlexDaniel
Copy link
Member

Personally, I am happy with this test. It went much smoother than I thought it would. In the end, I think, most contributors configured their editors to either show trailing whitespace or strip it automatically.

Those who didn't probably use make test as part of their workflow, which is good also.

This, however, cannot be said about edits made via GitHub. Indeed, that is a problem.

So while I don't mind the test becoming optional, I don't really understand what we should do next.

  • Should we leave trailing whitespace mess as is?
  • Should we clean it up periodically?
  • Should we write a bot that will do that automatically?
  • … ?

@zoffixznet
Copy link
Contributor Author

Should we write a bot that will do that automatically?

IMO that seems the simplest solution. Could be a simple bash script that runs on hack in a cron job.

@AlexDaniel
Copy link
Member

So should this test be turned off for travis only (making local make test fail by default) or should we just rely on the bot completely?

@AlexDaniel
Copy link
Member

As for the bot, I can do that.

@zoffixznet
Copy link
Contributor Author

zoffixznet commented Jul 22, 2016

Yeah, turning off for travis only is the best, I think. This way if a contributor has a chance to run make test, they can spot the issues before committing.

@coke
Copy link
Collaborator

coke commented Jul 22, 2016

Resolved in 4aee722 - 'make test' now skips whitespace test. 'make xtest' runs everything.

@coke coke closed this as completed Jul 22, 2016
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

No branches or pull requests

3 participants