-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove dependency on JDK 17 HexFormat #85
Conversation
Add ByteArray, ByteUtils, and HexFormat classes to do it.
7294fe7
to
e7635a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit unfortunate that this PR duplicates a couple of classes from base.internal
. But I guess since it's internal, secp
can't use it?
import java.util.Comparator; | ||
|
||
/** | ||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An empty comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now...
return ARRAY_UNSIGNED_COMPARATOR; | ||
} | ||
|
||
// In Java 9, this can be replaced with Arrays.compareUnsigned() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume you're planning for JDK 8? Otherweise you as per comment you could replace this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, planning for JDK 8!
I think the duplication is ok, for now. But I envision the following strategy:
|
There are definitely a few things in here that need cleanup, but I'm going to merge and we can fix them later. |
Add ByteArray, ByteUtils, and HexFormat classes to do it.