-
Notifications
You must be signed in to change notification settings - Fork 86
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 empty / error header constructor, put u32::MAX as size for compac… #84
Conversation
…t datalookup in case of error
@@ -33,6 +33,7 @@ pub enum Error { | |||
)] | |||
pub struct DataLookup { | |||
pub(crate) index: Vec<(AppId, DataLookupRange)>, | |||
pub(crate) is_error: bool, |
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.
What about making DataLookup
optional in the header instead of adding is_error
?
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.
Or even Result?
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'm not in favour of doing drastic changes again, any thoughts @markopoloparadox ?
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.
As discussed in the previous call, we cannot touch CompactDataLookup
for the time being. Making DataLookup an Option or Result will not yield the desired outcome since we need that struct to be valid in order to decode it into CompactDataLookup
.
IE it easier to add a flag and create a corresponding CompactDataLookup
out of it then to write special functions to handle Option<DataLookup>
into CompactDataLookup
conversion and vice versa
…t datalookup in case of error