Page MenuHomePhabricator

Migrating from FindBugs to SpotBugs
ClosedPublic

Authored by CLOVIS on Apr 24 2019, 9:27 AM.

Diff Detail

Repository
rLIBWFJVA WildFyre Java Libary
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Hackintosh5 added inline comments.
.gitlab-ci.yml
41

Do we really want spotBugs on tests?

api.sh
49 ↗(On Diff #879)

--norun should never start the api

build.gradle
78–79

don't use compile its deprecated
use format strings not concatenation

79

again, format strings please

src/test/Dockerfile
32 ↗(On Diff #879)

Better to use python3 -m pip; on some systems the alias is missing

This revision now requires changes to proceed.Apr 24 2019, 11:51 AM
.gitlab-ci.yml
41

Just looked at old stuff, Info Screen accepted D226 (closed cos out of date) which removed spotbugsTest in favour of spotbugsMain only

.idea/runConfigurations/Static_Analysis.xml
14

Here also should remove spotbugsTest

api.sh
81 ↗(On Diff #879)

again the & won't help. you have to remove exec if you want it to run in the background.

gradle/wrapper/gradle-wrapper.properties
2 ↗(On Diff #879)

You cant copyright the gradle wrapper configs!

src/test/Dockerfile
20 ↗(On Diff #879)

why --no-cache?

27 ↗(On Diff #879)

why? also this isnt the official repo...

CLOVIS added inline comments.
build.gradle
78–79

The FindBugs to SpotBugs migration post says to use compile

src/test/Dockerfile
20 ↗(On Diff #879)
27 ↗(On Diff #879)

Find that file in any official repo and I'll change the link

32 ↗(On Diff #879)

Doesn't matter here, it's a raw Alpine container there's no compatibility issues

build.gradle
78–79

Don't.
From https://stackoverflow.com/a/44419574/5509575:
Using api is the equivalent of using the deprecated compile, so if you replace all compile with api everything will works as always.

src/test/Dockerfile
27 ↗(On Diff #879)

Well the README of that repo says that it comes from mercurial at https://hg.openjdk.java.net/jdk8/jdk8/. I'd have thought it was fairly obvious.

32 ↗(On Diff #879)

True, but nicer to be as platform agnostic as possible

CLOVIS added inline comments.
src/test/Dockerfile
27 ↗(On Diff #879)

Yeah, that's the official repo. The file I need is not in there though, Oracle stopped distributing it.

The closer you can get to an "official" way of doing it is this: https://bgasparotto.com/extract-src-zip-jdk-installer

src/test/Dockerfile
27 ↗(On Diff #879)

What file do you need then?

src/test/Dockerfile
27 ↗(On Diff #879)
src/test/Dockerfile
27 ↗(On Diff #879)

That uses apt, which doesn't exist on Alpine.

Rebasing everything on master

WyldBot requested changes to this revision.May 19 2019, 1:41 PM
This revision now requires changes to proceed.May 19 2019, 1:41 PM
.gitlab-ci.yml
41

I don't know... code quality in the tests sounds important to me, even if they're not in prod

Can we get this or something similar in please, I've been trying for months and these deprecation warnings are annoying me

Herald added a required legal document: Restricted Legalpad Document. · View Herald TranscriptJul 1 2019, 9:42 AM
This revision is now accepted and ready to land.Jul 2 2019, 8:24 PM