activate sourcecode formatting

[mwiede]

This will use formatting rules of eclipse-java-google-style.xml to format the source code in maven process-sources phase.
There should be only whitespace changes in java files.

Checkstyle Plugin is added, but not used by default, since current code would have 2640 violations against Google-Style.

$ mvn validate -Pcheckstyle
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  23.848 s
[INFO] Finished at: 2022-11-29T10:34:30Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.2.0:check (validate) on project jsch: You have 2640 Checkstyle violations. -> [Help 1]

Cleaning these up is an additional effort.

[norrisjeremy]

Have you considered using fmt-maven-plugin?
It directly utilizes Google's google-jave-format library to process sources.
We typically add it's check goal like this to our projects so that non-conforming code causes a build failure during the verify phase:

      <plugin>
        <groupId>com.spotify.fmt</groupId>
        <artifactId>fmt-maven-plugin</artifactId>
        <version>2.19</version>
        <executions>
          <execution>
            <goals>
              <goal>check</goal>
            </goals>
            <phase>verify</phase>
          </execution>
        </executions>
      </plugin>

And to directly use it's format goal to reformat sources, you can manually execute mvn fmt:format.
It may require some additional tweaks to the plugin configuration to pick-up all the non-standard source directories where we put the > Java 8 source code.

[mwiede]

i have not compared fmt-maven-plugin, but as far as i can see formatter-maven-plugin offers the same. you can both format source code as well as verify it.

[mwiede]

as per github stats, fmt-maven-plugin is used by 185 repos and formatter-maven-plugin is used by 2.5k repos.

[norrisjeremy]

It appears formatter-maven-plugin also has a validate goal: is there a reason to to use maven-checkstyle-plugin instead of that?

Also, do we need to configure these plugins to also handle the > Java 8 code directories (src/main/java9, src/main/java11, etc.)?

[norrisjeremy]

I think we may want to also remove all these editor hints at the top of all the files too?

[mwiede]

yes makes sense

[norrisjeremy]

Did we want to handle removal of these editor hints in this PR or do that separately?

[mwiede]

I will add it to this one

[norrisjeremy]

Do we know what's causing the test failures during CI builds?

[mwiede]

Do we know what's causing the test failures during CI builds?

I looked at it. The problem seems to be the docker bind mount of the temporary directory. We need to move the temp directory to the current workspace.

[mwiede]

Do we know what's causing the test failures during CI builds?

I looked at it. The problem seems to be the docker bind mount of the temporary directory. We need to move the temp directory to the current workspace.

that was a problem I had within Github Codespaces environment. The other problem was now resolved by downgrading the runner image to ubuntu 20.04

[norrisjeremy]

I think we may want to add -Dformatter.skip=true in .github/workflows/maven.yml for the Test with Maven step.
I believe that will prevent Github CI from executing the formatter validation check when it executes the unit & integration tests (since it already would have done this during the prior Build with Maven step).

[norrisjeremy]

I think we may want to add -Dformatter.skip=true in .github/workflows/maven.yml for the Test with Maven step.
I believe that will prevent Github CI from executing the formatter validation check when it executes the unit & integration tests (since it already would have done this during the prior Build with Maven step).

Disregard, I missed that formatter:validate is tied to verify phase, so it has to happen during the Test with Maven step.

[norrisjeremy]

I think we should do a squash merge of this so it's a single commit.