-
Notifications
You must be signed in to change notification settings - Fork 15
Partial document struct for updates #53
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
Conversation
8e44901 to
365c353
Compare
365c353 to
6877bba
Compare
|
The partial struct derive is neat! I couldn't think of any good use cases for
id is not updatable |
e8b2664 to
eba2114
Compare
eba2114 to
bec629d
Compare
6bafd93 to
18f7f14
Compare
|
@haydenhoang check it out. Does it look good? |
|
Added a trick to comment an auto-generated code in mustache, if |
This fix is about vendor extension scoping problem
|
This looks good! But we need to re-add tests for the bulk documents operations: import, export, bulk update and bulk delete tests |
|
I did not notice that strongly-typed representation tests are different. I thought it's a copy of schemaless. Can you check them? I'm not that good at writing tests. I have left them commented in Also I'm thinking that maybe jsonl methods ( |
Yes, let's implement this in another PR. Will add those missing tests shortly. |
…e and bulk delete Also renamed `documents().export()` to `export_jsonl()`
|
This is ready for merge! |
|
Can't merge my own PR using the button. |
Auto-generates a partial document struct for updates. To avoid
serde_json::Valueas an input.Derive macro creates
struct BookPartialwith all optional fields.@haydenhoang what do you think? Need help with deciding about
collection_schemalesstests (intypesense/tests/client/documents_test.rs). Do you needcollection_schemalessupdates and creates? As you know I hateserde_json::Value. Is there any point in implementing it?Also I don't know if
idis updatable.