Remove trailing whitespace

[SaschaMann]

cc @ee7 @jiegillet

[ee7]

I've checked that, with this PR, the only remaining whitespace problem in this repo is line 90 in the corresponding file outside global-files (that file is updated when this PR is merged):

org-wide-files/.github/labels.yml

Line 90 in 5c9a6fe

#

Nice to see that Elixir CI failed too.

Some reasons that whitespace problems, although seemingly trivial, can nevertheless produce significant friction:

1. Many people configure their editor to trim trailing whitespace (and ensure exactly 1 final newline) on saving. It's annoying to see unrelated whitespace changes in the diff both when reviewing and committing ("can I be bothered to request/create a separate PR for this?")
2. Some people have git hooks that check whitespace, so a person making an unrelated change in a repo with a whitespace problem can see git commit fail.
3. When reviewing PRs that are most appropriately merged without squashing, it can turn a quick merge into back-and-forth "this looks good, but please amend commit x to remove the trailing whitespace, resolve the conflicts, and commit y to add a final newline, and force-push".
4. If whitespace isn't checked in CI, checking whitespace (and suggesting fixes) can be more time consuming for the reviewer than reviewing the actual intended diff.

[SaschaMann]

I've checked that, with this PR, the only remaining whitespace problem in this repo is line 90 in the corresponding file outside global-files (that file is updated when this PR is merged):

Thanks!

---

Imo this PR should be merged with skipped CI to not trigger a mass-PR to repos where it's not as important. Repos where the whitespace is causing issues can ignore the recent sync PR and manually request a rebuild, so far that'd be elixir and configlet.

[ee7]

Imo this PR should be merged with skipped CI to not trigger a mass-PR to repos where it's not as important. Repos where the whitespace is causing issues can ignore the recent sync PR and manually request a rebuild, so far that'd be elixir and configlet.

I'm fine with that. Can we also add nim and nim-test-runner, plus, say, problem-specifications and docs? I think the latter two would really benefit from whitespace checking too. Some repos probably have other whitespace problems anyway, and it's less painful because this file is not supposed to be manually edited downstream.

I think the main downside would be for a repo that previously had no whitespace problem, and has (or will have) a contributor with a whitespace-checking repo-wide git hook (who then rage-quits, because they can't even manually edit this file to fix the whitespace, and git commit -n is annoying).

But is it difficult to recreate the PR in every repo that didn't yet merge the previous one? I assume that we aren't set up to easily add a commit (or force-push) to the PRs that weren't yet merged.

[SaschaMann]

But is it difficult to recreate the PR in every repo that didn't yet merge the previous one? I assume that we aren't set up to easily add a commit (or force-push) to the PRs that weren't yet merged.

It's something that Erik might be able to do manually. You can't push to the PR branches, so you'd either have to manually enable that first or recreate the PRs from scratch.

We can always trigger a complete push afterwards if we want to but I'd rather do that when Erik is available so in the meantime merging it with CI skipped and manually triggering rebuilds is a hotfix for those repos where it's important.

I'm fine with that. Can we also add nim and nim-test-runner, plus, say, problem-specifications and docs?

Sure.

(who then rage-quits, because they can't even manually edit this file to fix the whitespace, and git commit -n is annoying).

They can edit it. The file itself is not protected in most repos, I think. You're not meant to manually edit the files because any changes would be overwritten but in this case if the change is identical to this PR, it would simply be ignored during the next org-wide sync.

[ErikSchierboom]

It's something that Erik might be able to do manually.

I can't do that, I don't have the permissions.

Imo this PR should be merged with skipped CI to not trigger a mass-PR to repos where it's not as important. Repos where the whitespace is causing issues can ignore the recent sync PR and manually request a rebuild, so far that'd be elixir and configlet.

Agreed.

[ErikSchierboom]

Could you verify that removing this line does not affect the generated labels.yml file? IIRC we had this empty line in here specifically for that reason.

[ErikSchierboom]

Two additional thoughts:

1. We should probably add the GHA workflow that checks the trailing whitespace in this repo too, to prevent this from happening again
2. I could fairly easily do a bulk PR myself once all the PRs have been merged to fix the trailing whitespace, without having to run the org-wide-files workflow. The benefit of this is that I can be certain that the PR only contains the trailing whitespace change.

[ee7]

We should probably add the GHA workflow that checks the trailing whitespace in this repo too, to prevent this from happening again

Created #205

[ee7]

I'll take the liberty of rebasing this on main.

Edit: I don't have permission :)

And rebasing isn't strictly necessary, because there's no merge conflict. Can we merge this? Skipping CI to avoid the mass PR?

[ErikSchierboom]

Approved