abstractj / kalium

Java binding to the Networking and Cryptography (NaCl) library with the awesomeness of libsodium
http://abstractj.github.com/kalium/
Apache License 2.0
207 stars 74 forks source link

Kalium 1.0.0? #48

Closed bendoerr closed 7 years ago

bendoerr commented 8 years ago

I started off with the goal of adding all of the currently documented Libsodium interfaces to Kalium. I may have gotten a bit carried away cleaning up code though. Anyway this is my progress so far, no need to merge yet.

Goals

abstractj commented 8 years ago

@bendoerr I'd say that would be better to split documentation and improvements in small PRs, when possible, it's better for visibility. Anyway is not a big deal, it will just take some time for a full review.

Regarding Kalium 1.0.0 or not. Let's leave it as is for now, more testing is required.

bendoerr commented 8 years ago

Sure, I'll split this up as much as I can.

I haven't had much time to work on it recently since I added the majority of the functionality that I needed. If I recall, I was blocked in one use case waiting for a bug fix to be merge into jnr-ffi/jffi... which looks like it has been merged and released now, so I'll include that in a future pull-request.

abstractj commented 8 years ago

@bendoerr thanks a million Ben

abstractj commented 8 years ago

@bendoerr here's my suggestion to get it merged. First, I really do appreciate your changes, but would like to do into this way.

1 I saw that you added several interfaces from libsodium and this is cool, at the same time I'd like to see the respective testing vectors for each interface (we can do it together in small steps). Otherwise Kalium would become just a Java library exposing plain C, the goal here is make it easy for people willing to consume crypto without having to care about what happens under the covers with libsodium.

Let's move to another PR the following commits please:

2 Let's get only the interfaces that requires an update and put in a single PR. Because from what I noticed we don't have the testing vectors for the new methods.

3 Let's add the remaining interfaces with testing vectors

Sorry if it sounds frustrating, I could just cherry-pick your commits, but I really would like to preserve what you did. If all that I said make some sense to you, let's move forward.

bendoerr commented 8 years ago

@abstractj not frustrating at all. Your suggestion sounds like a good approach and I appreciate the guidance. I'll get new PR's up later today.

bendoerr commented 8 years ago

@abstractj How do you feel about 733d3e6? I think it would resolve #37 and keep things more clear for those of us that are familiar with libsodium. Edit. See PR #58.

bendoerr commented 8 years ago

@abstractj I've added the two noted PR's let's figure out where to go with those, then I'll start popping up the rest.

abstractj commented 7 years ago

I'm closing it, because I believe this PR is out of date.