Page MenuHomePhabricator

Migrating from FindBugs to SpotBugs
ClosedPublic

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

Details

Summary

Update the Gradle Wrapper

Migrate from FindBugs to SpotBugs

Test Plan

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

CLOVIS created this revision.Apr 24 2019, 9:27 AM
Harbormaster completed remote builds in B648: Diff 879.
Hackintosh5 requested changes to this revision.Apr 24 2019, 11:51 AM
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

80

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
Hackintosh5 added inline comments.Apr 24 2019, 12:53 PM
.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
15

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 marked 2 inline comments as done.Apr 24 2019, 1:07 PM
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

Hackintosh5 added inline comments.Apr 24 2019, 1:14 PM
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 marked 2 inline comments as done.Apr 24 2019, 1:38 PM
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

Hackintosh5 added inline comments.Apr 24 2019, 2:22 PM
src/test/Dockerfile
27 ↗(On Diff #879)

What file do you need then?

Hackintosh5 added inline comments.Apr 24 2019, 2:33 PM
src/test/Dockerfile
27 ↗(On Diff #879)
CLOVIS added inline comments.Apr 24 2019, 3:07 PM
src/test/Dockerfile
27 ↗(On Diff #879)

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

CLOVIS updated this revision to Diff 925.May 15 2019, 8:58 PM

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
CLOVIS updated this revision to Diff 939.May 20 2019, 5:14 PM
CLOVIS added inline comments.May 20 2019, 5:15 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

Hackintosh5 accepted this revision.Jul 1 2019, 9:42 AM

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
WyldBot accepted this revision.Jul 2 2019, 8:24 PM
This revision is now accepted and ready to land.Jul 2 2019, 8:24 PM