refacto MemoryADT implementation for KmsEncryptionLayer#46
Conversation
HatemMn
left a comment
HatemMn
left a comment
There was a problem hiding this comment.
Looks good ! the few comments are just form, I think this will stop the stack overflowing
|
|
||
| pub(crate) fn findex_number_of_threads() -> Option<usize> { | ||
| std::env::var("GITHUB_ACTIONS").is_ok().then_some(1) | ||
| std::env::var("GITHUB_ACTIONS").map(|_| 1).ok() |
There was a problem hiding this comment.
while 100% equivalent the old syntax was more verbose and hence easier to understand for me but feel free to close this if you prefer the new one
There was a problem hiding this comment.
This is a concurrent modification I kept through the rebase. I guess which version is more readable depends on personal taste. I for example feels the other version to be less straightforward since it involves more Rust-specific helper functions (I don't know by heart the signature of is_ok(), while map is standard).
I can revert this change if you prefer.
This is a pure refacto and I do not expect it to solve the stack overflow issue at all. |
|
all good, we can merge |
3 participants
An arguably simpler version of the
MemoryADT:IntoIteratorfor an option to merge the two branches of the if inguarded_writebatch_read