Skip to content
This repository was archived by the owner on Apr 4, 2023. It is now read-only.

Conversation

@irevoire
Copy link
Contributor

@irevoire irevoire added bug Something isn't working API breaking The related changes break the milli API labels Jan 24, 2023
@irevoire irevoire requested a review from loiclec January 24, 2023 11:21
@irevoire irevoire force-pushed the error-on-extra-field-in-geo branch from 36c2a47 to de3c4f1 Compare January 24, 2023 11:23
Copy link
Contributor

@loiclec loiclec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank youuuu :)

documents!({ "id" : "doggo", "_geo": { "lat": 1, "lng": 2, "doggo": "are the best" }}),
)
.unwrap_err();
insta::assert_display_snapshot!(err, @r###"The `_geo` field in the document with the id: `"\"doggo\""` contains the following unexpected fields: `{"doggo":"are the best"}`."###);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`{"doggo":"are the best"}`

This could be a bit confusing, because this object appears nowhere in the original document. I don't know what the alternative should be though, and it's just an error message, so not a big deal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I thought of displaying only a list of field name, but then I found it a bit confusing as well when it comes to more complex values 😩

I guess we’ll wait and see, if people complains about this message we’ll change it.

@irevoire
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 24, 2023

Build succeeded:

@bors bors bot merged commit 4e4d8df into main Jan 24, 2023
@bors bors bot deleted the error-on-extra-field-in-geo branch January 24, 2023 12:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

API breaking The related changes break the milli API bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants