Create initial linter for z390 java code - #806
Conversation
Zaneham
left a comment
There was a problem hiding this comment.
Hey Abe, I've had a proper look at this one. The ruleset itself looks good. One thing that isn't inline anywhere because it's about what's missing rather than what's here, the linter only runs from bat/BLDJAR.BAT. CI builds on ubuntu-latest through bash/blddist and bash/bldjar, so nothing here ever fires on a PR. That means the cleanup in 895-898 can regress without anyone noticing, and contributors on Linux never see it at all. Should bash/bldjar get the same block, or would you rather it was its own CI job?
| java -jar build\checkstyle\checkstyle-all.jar -c src\checkstyle.xml src | ||
| if %ERRORLEVEL% NEQ 0 goto return |
There was a problem hiding this comment.
This runs over the whole src tree before javac, so merging this on its own stops the Windows build dead. I ran checkstyle 13.9.0 with your config over src at this head and got 108,938 violations across 20 files, mostly pz390, mz390, az390 and sz390. Even once 895-898 are in, about 93,000 are left.
So either this goes last, or the gate wants to be warn-only for now. A suppression file listing the not-yet-clean modules would let it be a real gate from day one and shrink as you work through them.
| $ProgressPreference = 'SilentlyContinue' | ||
|
|
||
| # Request latest release information | ||
| $apiUrl = 'https://api.github.com/repos/checkstyle/checkstyle/releases/latest' |
There was a problem hiding this comment.
releases/latest means whoever builds gets whatever happened to be current when they first ran it, and a new checkstyle release can fail the build with no code change. Latest is already 13.9.0 against the 13.4.2 in your comment above. You write version.txt on line 30 but nothing ever reads it, so pinning a version and comparing against that file would fix both at once.
| Write-Progress -Activity "Download" -Completed | ||
| Write-Host "Download failed: $($_.Exception.Message)" -ForegroundColor Red | ||
| exit 1 | ||
| } No newline at end of file |
There was a problem hiding this comment.
No trailing newline on this one.
|
Hi @Zaneham, You are correct - this is still a work in progress. It started out as a minimal thingy, just to prove feasibility. Then got out of control more or less. So, yes, the plan is indeed to fix all the java programs before merging this change. And an update to the bash build script definitely will have to be part of that merge. That's the whole point of having a linter to begin with. |
fixes #807
For now: only 1 check (for unused imports) and only in the local build on windows.
Making a comparable change to the bash/bldjar process is okay.
But we must make sure the build process on github remains unaffected until we resolve the errors reported ...