Page MenuHomePhabricator
Differential D2681

[secp256k1] remove guava dep
ClosedPublic
Actions

Authored by floreslorca on Mar 12 2019, 00:53.

Details

Reviewers
deadalnix
Fabien
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rSTAGING31f5ed8c61ea: [secp256k1] remove guava dep
rABC31f5ed8c61ea: [secp256k1] remove guava dep
Summary

remove overkill depencency to guava

Test Plan

execute previous tests successfully

the JNI part can be built and tested by simply doing

$ ./autogen.sh
$ ./configure --enable-jni --enable-experimental --enable-module-ecdh
$ make check-java

Once https://reviews.bitcoinabc.org/D2686 is passed we can do this automatically

Diff Detail

Repository
rABC Bitcoin ABC
Branch
guava
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5207
Build 8477: Bitcoin ABC Buildbot (legacy)
Build 8476: arc lint + arc unit

Event Timeline

floreslorca created this revision.Mar 12 2019, 00:53
Owners added a reviewer: Restricted Owners Package.Mar 12 2019, 00:53
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 12 2019, 00:53
Herald added a reviewer: deadalnix. · View Herald Transcript
Herald added a subscriber: schancel. · View Herald Transcript
teamcity added a subscriber: teamcity.Mar 12 2019, 00:54

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

floreslorca edited the test plan for this revision. (Show Details)Mar 12 2019, 01:05
floreslorca edited the test plan for this revision. (Show Details)
teamcity added a comment.Mar 12 2019, 01:09

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5207: Diff 7682.Mar 12 2019, 01:09
deadalnix requested changes to this revision.Mar 12 2019, 20:01

Can we get the java tests fixed before changes are made rather than mix the two together ?

This revision now requires changes to proceed.Mar 12 2019, 20:01
deadalnix added a comment.Mar 12 2019, 20:01

Also, adding a test of the java binding to CI would be beneficial so that we can catch these things.

floreslorca edited the test plan for this revision. (Show Details)Mar 13 2019, 20:34
floreslorca added a comment.Mar 13 2019, 20:44

depends on these

https://reviews.bitcoinabc.org/D2685
https://reviews.bitcoinabc.org/D2686

floreslorca requested review of this revision.Mar 20 2019, 06:09
Fabien added a subscriber: Fabien.Mar 20 2019, 13:24

There seem to be several things in this diff, one is removing the guava dependencies and the others are refactoring (indent and boolean assignations).
You should consider splitting the diff.

floreslorca updated this revision to Diff 7769.Mar 21 2019, 04:40

remove indentation

teamcity added a comment.Mar 21 2019, 04:41

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Mar 21 2019, 04:55

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5264: Diff 7769.Mar 21 2019, 04:55
Fabien requested changes to this revision.Mar 21 2019, 08:57

There is some mix with the previous version in the diff, it does not display the difference from master. Could you please rebase on top of master ?

This revision now requires changes to proceed.Mar 21 2019, 08:57
floreslorca updated this revision to Diff 7796.Mar 22 2019, 20:32

rebase master

teamcity added a comment.Mar 22 2019, 20:32

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

floreslorca added a comment.Mar 22 2019, 20:34

huh i did rebase but it still not showing the proper diff

Fabien added a comment.Mar 22 2019, 20:36

Did you rebased a single commit ?

teamcity added a comment.Mar 22 2019, 20:47

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5278: Diff 7796.Mar 22 2019, 20:47
floreslorca updated this revision to Diff 7798.Mar 22 2019, 21:15

remove guava dep in a single commit

teamcity added a comment.Mar 22 2019, 21:16

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Mar 22 2019, 21:31

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5280: Diff 7798.Mar 22 2019, 21:31
floreslorca updated this revision to Diff 7801.Mar 22 2019, 22:44

remove guava in build script

teamcity added a comment.Mar 22 2019, 22:45

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Mar 22 2019, 23:00

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5281: Diff 7801.Mar 22 2019, 23:00
Fabien accepted this revision.Mar 25 2019, 10:48
deadalnix added inline comments.Mar 25 2019, 16:36
src/secp256k1/src/java/org/bitcoin/NativeSecp256k1Util.java
42 ↗(On Diff #7801)

AssertFailException seems like a more appropriate exception.

deadalnix accepted this revision.Mar 25 2019, 16:38
deadalnix added inline comments.
src/secp256k1/src/java/org/bitcoin/NativeSecp256k1Util.java
42 ↗(On Diff #7801)

Never mind, the name makes it explicit this is about arguments.

This revision is now accepted and ready to land.Mar 25 2019, 16:38
Closed by commit rABC31f5ed8c61ea: [secp256k1] remove guava dep (authored by floreslorca). · Explain WhyMar 25 2019, 18:13
This revision was automatically updated to reflect the committed changes.
Fabien mentioned this in D3834: [TRIVIAL] Cleanup the JNI test file.Aug 13 2019, 06:48
floreslorca added a commit: rSTAGING31f5ed8c61ea: [secp256k1] remove guava dep.Sep 19 2019, 14:52

Revision Contents
Changeset List

PathSize
src/
secp256k1/
6 lines
13 lines
src/
java/
org/
bitcoin/
23 lines
123 lines
6 lines

Diff 7682

View Options

src/secp256k1/.travis.yml

Loading...
View Options

src/secp256k1/Makefile.am

Loading...
View Options

src/secp256k1/src/java/org/bitcoin/NativeSecp256k1.java

Loading...
View Options

src/secp256k1/src/java/org/bitcoin/NativeSecp256k1Test.java

Loading...
View Options

src/secp256k1/src/java/org/bitcoin/NativeSecp256k1Util.java

Loading...