-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add extractParams for Argon2, PBKDF2, and Scrypt #61
Conversation
The |
Not sure why, but |
(The reason When if the |
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.
Please also add an entry into the ChangeLog.md
and bump the version in the password.cabal
file.
Looks good! Thank you for your effort! If you want to just be done with this PR, that's fine too, I'll add the tests when I get some time to do so and we'll merge/upload after that. |
I've also just realized the And also that the first parameter when splitting on Again, if you don't feel like doing this, I can finish up the PR if you want to. |
For the tests, I'm willing to do it, but I need some guidance (I already adapted some tests, but I have no idea on new relevant tests). |
Ok, that's great! The additions to the test are ok, but I'd rather you add another parameter that will be the expected I'd also like to add a few property tests to check the following:
Those together with what you already added in the tests should be enough for this addition, I think. |
The testProperty "newSalt <-> length salt" $
\i -> ioProperty $ do
let n = fromIntegral (i :: Int16)
Salt salt <- newSalt n
pure $ Data.ByteString.length salt === n (casting the EDIT: On second thought... |
I did, however:
|
Ah, right... now I remember why we didn't have Can you give the error that you get when the Now lastly, I'd just like to get an extra copy of every first |
Here are the failing tests (tasty):
|
I wonder why I wonder why |
Yep, it works without the condition |
Oh, wait, the |
Ýou'll have to set the I also get the feeling the |
I think there's a real bug here. I have tried to change |
Thank you for all the effort and patience 🎉 |
Thanks for helping me, it was great. |
I'm tackling #55