Add automated Java code review

[NicMcPhee]

I think that @floogulinc plans to add ng lint to the set of automated checks here (a Good Idea), and we should also probably add some sort of Java check. There's an "Run java checkstyle" app that looks like it would do The Right Thing, although it requires a secret GITHUB_TOKEN to work, so it's a little more work than just adding the job to the YAML file.

It's also based on reviewdog, which clearly supports about a bazillion checks, including four on text, a docker check, shellcheck for shell scripts, stylelint for CSS, yamllint, etc., etc. We could clearly do a lot of digging around there and find a bunch of other things we could add.

[floogulinc]

It would probably make sense to just integrate checkstyle into our gradle config so students can also run it locally: https://docs.gradle.org/current/userguide/checkstyle_plugin.html

(Also, the GITHUB_TOKEN secret is automatically provided to each Actions run)

[floogulinc]

I would guess this is checking many of the things lgtm already is, would there be too much overlap for it to matter?

[NicMcPhee]

Good question. I'm not sure they are checking the same things. It looks like LGTM is actually checking less syntactic things and more "high level" concerns. At least a quick look over their queries, it's not clear that they check things like method names using camel case starting with a lowercase letter, and class names starting with an uppercase letter. This is a pretty common mistake in Software Design, and just makes you look like you don't really know what you're doing (would suck in an interview), so it would be nice to have it checked somewhere.

I like the idea of just using Gradle, though, as opposed to a fancy GitHub Actions thing.

[floogulinc]

Ah I see, so this is more like we we have TSLint doing, enforcing the opinionated specifics of code style and formatting.

[NicMcPhee]

Yup. When we used to use Java really heavily, students came out of our program really knowing it well. We use it much less now, so students are often much weaker on a host of things. We can't fix all of those in 3601, but we can at least make sure sure they're using the industry standards for naming and such.

[NicMcPhee]

We experimented with this, but it turns out that checkstyle generated a lot of noise so it wasn't clear if we really wanted to go down this road. If we did, we'd probably need to provide a fairly tuned configuration to turn off/down a bunch of rules that we're not overly concerned about.