review: checkstyle: Spotless configuration

[micka-lama]

close #5741

I update the configuration of the Spotless plugin. The existing configuration was too specific (the scan was limited to src/test/java/spoon/testing/assertions/**/*.java). The new one is global (only test classes are excluded, **/testclasses*/**).

The order of the imports is:

non-static
<blank>
static javax
static java
<blank>
static *

I also apply the IntelliJ basic style about the indentation (4 spaces) and whitespaces are trimmed.

Spotless documentation: https://github.com/diffplug/spotless/tree/main/plugin-maven

[I-Al-Istannen]

I haven't looked at it, but you need to preserve complete formatting for the generated assertions. It was limited to assertions deliberately: The generated tests do not follow a consistent formatting, so the spotless run is required to prevent changes on every execution. If you remove that, the generated code CI will fail on you. Just re-ordering import statements will not cut it for the generated code.

[micka-lama]

Got it! Thank you

So the goal is to not have a git diff after applying Spotless on the generated classes? Is it ok for you if I commit with the new Spotless configuration only on the generated classes?

Initially, I didn't want to modify a lot of files here, just thepom.xml.

[I-Al-Istannen]

So the goal is to not have a git diff after applying Spotless on the generated classes?

Yes, exactly. The import order stuff (and also general formatting, I guess) would be nice for all of spoon. The generated assertions need to be completely managed by spotless though, so there is no diff after generation.

Is it ok for you if I commit with the new Spotless configuration only on the generated classes?

Sounds fine for a second commit in this PR, yea.

[micka-lama]

The import order stuff (and also general formatting, I guess) would be nice for all of spoon.

My mistake, you are right. I need to commit all the result of mvn spotless:apply (not only on src/test/java/spoon/testing/assertions/**/*.java). Otherwise, there will be a difference on the other part of the spoon project for the git diff.

[I-Al-Istannen]

If the reformat looks good and you are happy (and tests pass and the integrators are happy), you should create a .git-blame-ignore-revs file in the root of the repository with the commit hash of the reformat commit on its only line. I.e. if the hash of the reformat commit is c8481bc206ea28c0b7a5e88a64314e1ecb3179ce, you would create a .git-blame-ignore-revs file with the following content:

c8481bc206ea28c0b7a5e88a64314e1ecb3179ce

This will cause GitHub, git blame and most IDEs to ignore this commit when calculating blames. Makes the history a lot more usable, otherwise everything will be attributed to you.

[micka-lama]

Because the strategy is a squash, the commit hash will be different compared to my local branch. It should be done in at least two PR, right? One, here, with the reformatting and another one with the .git-blame-ignore-revs file containing the hash of the squashed commit.

[algomaster99]

It should be done in at least two PR, right?

Good catch. You could brute force a commit hash, but that's overkill here. The second PR would contain the commit hash of the squashed commit.

[I-Al-Istannen]

Are you happy with this PR, i.e. can I take a look at it? :)

[micka-lama]

Are you happy with this PR, i.e. can I take a look at it? :)

Yes everything should be ok now! I am waiting for an eventual squash to create a second PR with the .git-blame-ignore-revs file.

[I-Al-Istannen]

I ported your changes onto the head of master to fix conflicts and separated configuration and formatting in separate commits (so it is easier to change in the future, if necessary).

Looking at the diff, I am also not sure if the formatting is a benefit or drawback. It also seems quite hard to get IJ to reproduce it.

WDYT @SirYwell @MartinWitt?

[micka-lama]

Hello, any updates? It's been a while since the last activity. Please let me know if there's anything I can do.