-
Notifications
You must be signed in to change notification settings - Fork 89
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
Use salt as an array of bytes instead of String #18
Conversation
Hi there, First, congrats on you first pull request! Your request points out a missing API in the current library (thank you!), one that accepts the salt as an array of bytes, but your solution, rather than adding this capability, replaces an existing one. For that reason, I am not going to merge the request "as is". Instead I will add the new API alongside the existing one, or if you would like to modify your pull request so that it does this, I will merge it. I hope this is clear, let me know if you have any questions. Cheers, |
I see your point. If you give me some time, I make the changes myself and request a new pull. |
Great. Look forward to the new pull request. I'll leave this open for the moment as a reminder of the issue and close this and the new request together. |
Recover previous derivedKey method with String salt for backwards compatibility
I pushed the previous code to my branch, and it showed in this pull request. Do I have to create a new one? |
No, you don't have to create a new one. Just one more thing and this can be merged. You need to test both methods. Your previous commit removed tests for the Thanks for your help on this. I think this is a useful addition to the library! |
You're right. I'll add the tests as soon as I get some time, hopefully in a couple of days. |
Hi andredsp, |
Hi Danny, |
Do you need any other action on my side? |
Hi Andre, |
Use salt as an array of bytes instead of String. Also added arrayFromString to ease use of existing code samples.
Note: it's my first pull request. Sorry if I'm messing something.