-
Notifications
You must be signed in to change notification settings - Fork 7
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 the core of the ordinary index support #244
Conversation
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.
This adds support for the ordinary index apart from serialization.
Another missing feature is incremental construction, or is that included in the serialisation?
Are you planning on adding tests in a follow-up PR?
I understood that serialization is supposed to be incremental and thus incremental construction is entangled with serialization. Consequently, I wanted to include incremental construction in the serialization part. Did I get something wrong here?
The testing issue is an interesting one. My understanding is that there should ultimately be a common interface for indexes, with instances for the compact and the ordinary index. Tests could and perhaps should then be written for indexes as such, using said interface, and run for the different instances. My impression is that adding tests specifically for the ordinary index would merely boil down to copying compact-index tests, which wouldn’t seem like a good idea to me. What are your thoughts about this? |
Sure, that's true, the two are closely related, but not exactly the same. I was just wondering why you mentioned that only serialisation was missing
Having at least one simple property test to give us some assurance that |
I’ve filed issue #265 to deal with this. |
a56b90e
to
378a844
Compare
378a844
to
4d69696
Compare
This adds support for the ordinary index apart from serialization.