search

exercism / org-wide-files

MIT License
7 stars 8 forks source link

consider adding org-wide workflow to lint whitespace #213

Open ee7 opened 2 years ago

ee7 commented 2 years ago

Some arguments in https://github.com/exercism/org-wide-files/pull/204#pullrequestreview-950940243.

And from e.g. https://www.kernel.org/doc/html/v4.10/process/coding-style.html

Do not leave trailing whitespace at the ends of lines. Some editors with smart indentation will insert whitespace at the beginning of new lines as appropriate, so you can start typing the next line of code right away. However, some such editors do not remove the whitespace if you end up not putting a line of code there, such as if you leave a blank line. As a result, you end up with lines containing trailing whitespace.

Git will warn you about patches that introduce trailing whitespace, and can optionally strip the trailing whitespace for you; however, if applying a series of patches, this may make later patches in the series fail by changing their context lines.

If we wanted to add whitespace linting, we could do it in configlet lint. But then we'd only lint whitespace in repos where configlet lint runs. So it seems an org-wide workflow is better.

SaschaMann commented 2 years ago

If we add this workflow, we should ensure that all tracks pass the check through a mass PR beforehand.

Otherwise this will likely cause frustration due to introducing a failing CI check that requires a manual fix.

ee7 commented 2 years ago

I agree.

ErikSchierboom commented 2 years ago

Personally, I'm not in favor of adding such a workflow. To me, the benefits are quite small whereas it is likely that (new) contributors would see their PRs failing CI for trailing whitespace which likely doesn't influence the behavior in the repo. If everyone thinks it is a good idea, I'll surely reconsider my position.

SaschaMann commented 2 years ago

I've still not read a compelling reason why trailing or extra whitespace is bothersome. I understand that in practice, having it will cause issues for tools, but I have yet to understand why the tools work that way in the first place.

ee7 commented 2 years ago

(I don't have a strong opinion yet about whether I'd want to force this on repos).

I have yet to understand why the tools work that way in the first place.

A big one is that git is designed for email, and email does not play nicely with whitespace problems.

Many serious projects, like the Linux kernel (which git was written for) and git itself use email-driven workflows. The GitHub model is not the original one.

Many of the arguments that Linus (the creator of git) makes in e.g. torvalds/linux#17 (strong language!) and here are still valid. Some of the problems don't apply to us, but "no trailing whitespace" absolutely is a best practice.

A tutorial for using git in email-driven workflows: https://git-send-email.io/

ErikSchierboom commented 2 years ago

Many serious projects, like the Linux kernel (which git was written for) and git itself use email-driven workflows. The GitHub model is not the original one.

But that doesn't apply to us then? We are solely using GitHub, and I would prefer people to use Github and not email to collaborate.