Skip to content
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

Implement FromHex for 32-byte arrays #13

Merged
merged 2 commits into from Nov 13, 2017
Merged

Conversation

illicitonion
Copy link
Contributor

No description provided.

@LukasKalbertodt
Copy link
Contributor

Hello and thanks for your PR! Adding one specific array length begs the question: why not other lengths? We could, for example, add all multiple of 8 below or equal to 64 (24, 32, 40, 48, 56, 64). I'm not sure what's the right solution here. Any other opinions?

@illicitonion
Copy link
Contributor Author

I'm happy to add multiples of 8 up to some value, I just had a need for 32 because it's a pretty common digest algorithm output size (and one I'm using).

Longer-term, I think rust-lang/rust#44580 means we should be able to make this generic in the future (its RFC got accepted, and someone is interested in working on it).

@LukasKalbertodt
Copy link
Contributor

Yip, absolutely, the current situation is just a dirty hack. Anyway, I'm not the maintainer of this crate, so you' (we) will have to wait for @KokaKiwi to decide what numbers to add ^_^

@KokaKiwi
Copy link
Owner

Hi, long time I haven't been active here (kind of a chaotic schedule recently, in my head too ^^")
I think of adding some sizes to implement, but that's purely arbitrary at the moment (as Lukas said, that's just dirty hack in waiting for the RFC): 24, 48 and 64 (multiples of 16 between 16 and 64 seemed to be a good thing to me)

@illicitonion
Copy link
Contributor Author

Done :) Thanks!

@KokaKiwi KokaKiwi merged commit 65f9ee8 into KokaKiwi:master Nov 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants