Page MenuHomePhabricator

Update gradle wrapper and fix dependencies. Fixes T331.
AbandonedPublic

Authored by Hackintosh5 on Feb 28 2019, 2:22 PM.

Details

Reviewers
CLOVIS
WyldBot
Maniphest Tasks
T331: Kotlin-based projects cannot use libwf-java
Required Signatures
Restricted Legalpad Document
Summary

Update gradle wrapper and fix dependencies.

Test Plan

Unit tests and see if it intergrates correctly with a kotlin project

Diff Detail

Repository
rLIBWFJVA WildFyre Java Libary
Branch
arcpatch-D223
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 657
Build 657: arc lint + arc unit

Event Timeline

I don't understand what you're trying to do with the resource handling.

src/test/java/net/wildfyre/http/RequestTest.kt
38

Why?

114

Why?

127

Why?

149

Why?

src/test/java/net/wildfyre/http/RequestTest.kt
38

SpotBugs is the new version of FindBugs and it gets angry otherwise.

114

See above. FindBugs says that its a security risk because it allows access to files via an object (idk why its dangerous but it says that)

127

See above

149

See above

src/main/java/net/wildfyre/http/IssueInTransferException.kt
87–88

Again, spotbugs doesnt like the redundant !! (the whole line is in a json != null)

src/main/java/net/wildfyre/http/IssueInTransferException.kt
87–88

I don't know why arc let me push this, it doesnt compile...

src/main/java/net/wildfyre/http/IssueInTransferException.kt
87–88

Isn't json a bar? If so, this shouldn't compile

src/test/java/net/wildfyre/http/RequestTest.kt
114

The way it was before because there is no redundancy of the paths. Also, it is cleaner.

Sometimes, tools cannot know what is better. There are not here so we follow them blindly, but so they help find errors.

src/main/java/net/wildfyre/http/IssueInTransferException.kt
87–88

I dont even know any more. I'm going to rewrite that bit with easier to ask for forgiveness than permission, because thats the only way that the kotlin compiler, you, and spotbugs can all be happy at the same time.

  • Make SpotBugs sad :(
  • Make SpotBugs ignore security errors in the RequestTest

Made spotbugs ignore these security errors in the tests. In IssueWithTransferException, it is absolutely correct, that could *in some extreme cases where someone is abusing the API*, cause a crash.

src/main/java/net/wildfyre/http/IssueInTransferException.kt
90

This looks like programming by exception, which is not really a good practice

Maybe something like this could work better?

return json?.let {
  "${super.message}\n${it.toString(WriterConfig.PRETTY_PRINT)}"
} ?: super.message
WyldBot requested changes to this revision.Mar 27 2019, 3:35 PM
This revision now requires changes to proceed.Mar 27 2019, 3:35 PM

Remove programming by exception

It looks like the changes here are mainly the same as D247... I know this one was made earlier but D247 is up to date with master and with the new CI system so I think it should be prioritized (unless there's something important here)

In D223#6352, @CLOVIS wrote:

It looks like the changes here are mainly the same as D247... I know this one was made earlier but D247 is up to date with master and with the new CI system so I think it should be prioritized (unless there's something important here)

Herald added a required legal document: Restricted Legalpad Document. · View Herald TranscriptJul 1 2019, 9:56 AM