Page MenuHomePhabricator

[TRIVIAL] Cleanup the JNI test file
ClosedPublic

Authored by Fabien on Aug 9 2019, 10:38.

Details

Summary

Cleanup the NativeSecp256k1Test.java file:

  • Move comments on their own lines;
  • Add period at the end of sentences;
  • Remove extra whitespaces in parenthesis to make the code consistent;
  • Add/Remove some newlines where needed;
  • Remove commented out debug code;
  • Replace tabs with spaces;
  • ...
Test Plan
./contrib/teamcity/build-secp256k1.sh

Diff Detail

Repository
rABC Bitcoin ABC
Branch
secp256k1_cleanup_java_test
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 7093
Build 12232: Bitcoin ABC Buildbot (legacy)
Build 12231: arc lint + arc unit

Event Timeline

jasonbcox requested changes to this revision.Aug 9 2019, 15:45
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/secp256k1/src/java/org/bitcoin/NativeSecp256k1Test.java
13

mixed comment types

196

Kind of weird to append a period on the end of a link. maybe do this instead?

* It tests the following test vectors:
* https://github.com/sipa/bips/blob/bip-schnorr/bip-schnorr/test-vectors.csv
This revision now requires changes to proceed.Aug 9 2019, 15:45

Update as per comment.
Add a newline between the TODO and the javadoc comments for better readability.

Are you really confident you want to take ownership of this?

@deadalnix It's already the case, see D2681 or D2755. Also I have D3835 that will take ownership even more, but make your life easier for testing with buster.

This revision is now accepted and ready to land.Aug 13 2019, 21:38
In D3834#90773, @Fabien wrote:

@deadalnix It's already the case, see D2681 or D2755. Also I have D3835 that will take ownership even more, but make your life easier for testing with buster.

D3835 clearly have happened one way or another already: https://github.com/bitcoin-core/secp256k1/blob/master/src/java/org/bitcoin/NativeSecp256k1Test.java

@deadalnix Absolutely, sepc256k1 relies on BaseEncoding from the guava dependency which was removed from our codebase by D2681, hence the custom function in D3835.

This revision was automatically updated to reflect the committed changes.