White space config

[casperdcl]

• fix .clang-format files regex
• add CI check (see https://github.com/casperdcl/STIR-1/runs/1295351219#step:5:53)
• run clang-format (6.0) and commit as bot user

[KrisThielemans]

thanks, first time I see some actual GitHub actions. Seems quite nice. Not sure what this action is supposed to test. I guess it'll fail if the formatting of a PR is bad?

My feeling was to split this into the "white space fix" into 2 PRs: one that sets everything up, next one that actually ran clang-format. That way, people can first merge the config in their set-up/branch without having 1900 files changed. Bad idea?

[casperdcl]

it'll fail if the formatting of a PR is bad

exactly. Actions are great - lower overhead and less configuring that e.g. Travis. Also if you're very careful you can run GPU & MATLAB tests on your own machine :P

people can first merge the config in their set-up/branch without having 1900 files changed. Bad idea?

Doesn't make sense to me... people would indeed have to run pre-commit/clang-format and re-commit 1900 changes themselves. It seems better that the merge does this for them, possibly triggering a few conflicts which they'll have to manually fix.

[casperdcl]

P.S. we could also make the action commit the changes as a bot directly to the PR (so people won't even have to install pre-commit/clang-format themselves) but maybe that's a task for some other time.

[KrisThielemans]

it'll fail if the formatting of a PR is bad

exactly.

ok. should we/you rename the action then from Test? (no idea really)

[KrisThielemans]

people can first merge the config in their set-up/branch without having 1900 files changed. Bad idea?

Doesn't make sense to me... people would indeed have to run pre-commit/clang-format and re-commit 1900 changes themselves. It seems better that the merge does this for them, possibly triggering a few conflicts which they'll have to manually fix.

ok, I see that it'd be terrible to have multiple people commit 1900 changed files!

I'm still worried about the conflicts though. some of our PRs are very big (especially the TOF one). I guess I'm looking for a way that conflicts are only flagged up after running clang-format. That'd be easier if it's run first/during the merge. Not sure if this can be done.

[casperdcl]

just rebased on your modified .clang-format. git's merge ability is meant to sort out most conflicts and is meant to be a better strategy than all PRs running pre-commit changing 1k+ files before merge.

If there are a couple of major PRs then those can be exceptions where you:

• merge in the commit just before the one which touches 1k+ files
• run pre-commit run --all-files
• commit with --author="clang-format <[email protected]>"
• merge in master (should be minimal conflicts)

[casperdcl]

alternatively we could wait until after the big PRs are merged

[KrisThielemans]

ok.

I'll try this to merge locally first and see what happens, but it won't be straightaway.

thanks!

[KrisThielemans]

hi @casperdcl I'll merge this PR, but would you be able to remove the last commit (where you actually ran it)? we'll do this.

I guess I could do it myself by force-pushing to your branch or so, but not 100% sure...

thanks!

[casperdcl]

updated (original branch https://github.com/casperdcl/STIR-1/tree/white-space-config-orig)

[KrisThielemans]

thanks!