-
Notifications
You must be signed in to change notification settings - Fork 52
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
Refactoring phase 1: #94
Refactoring phase 1: #94
Conversation
42715c5
to
622bb91
Compare
I need some reviews regarding this. It is rather massive. |
Will have a look today, thanks 💯 |
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.
Thanks for the great effort! I really like the module tree, I'm guessing you got inspiration from the spec? Looking forward to potentially splitting the Context
implementation in multiple files as well.
I've left some comments below, but I don't think I reviewed everything thoroughly enough, might leave some more comments later.
Yeah I looked to the structures part of the spec and tried to basically copy it hehehe. |
ce0dd19
to
68ac55f
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.
Wow! Amazing changes, thanks a lot 🚀 That is going to make that crate way more easy to develop with!
Just a few comments.
de1e340
to
bdaca0d
Compare
- Moved context to its own file. - Moved algorithm specifiers into own directory. - Created new file for algorithm structure. - Created new for structures. - Created sub dir to structures for buffer types. - Moved Digest into buffers. - Added MaxBuffer to buffers. - Added Auth to buffers. - Added Data to buffers. - Added Nonce to buffers. - Created sub dir to structures for name types. - Added Name type to names. - Made use of these new types in the places I could find they where being used. Signed-off-by: Jesper Brynolf <jesper.brynolf@gmail.com>
bdaca0d
to
55c9067
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.
Thank you!
@@ -0,0 +1,51 @@ | |||
use std::convert::TryFrom; |
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.
Just as a note, not necessary for this PR - we might end up moving these tests back into the files where these types are defined. For Context
tests they need to be done as integration tests, but if nothing external to the test is needed then it's ok for them to be in the same file as the code they test.
} | ||
|
||
impl Nonce { | ||
const MAX_SIZE: usize = 64; |
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 like these, very neat - can they be accessed outside this module? Or, could they be made public? I think it'd be useful for users
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.
Currently the cannot be acessed outside the module bit ut is no problem changing tjat.
This is part of issue #87
Moved context to its own file.
Moved algorithm specifiers into own directory.
Created new file for algorithm structure.
Created new directory for structures.
Created sub directory to structures for buffer types.
Moved Digest into buffers.
Added MaxBuffer to buffers.
Added Auth to buffers.
Added Data to buffers.
Added Nonce to buffers.
Created sub directory to structures for name types.
Added Name type to names.
Made use of these new types in the places I could find they
where being used.
Signed-off-by: Jesper Brynolf jesper.brynolf@gmail.com