Skip to content

Properties should be camel case #1204

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 22, 2021
Merged

Properties should be camel case #1204

merged 1 commit into from
Jan 22, 2021

Conversation

darthf1
Copy link
Contributor

@darthf1 darthf1 commented Nov 2, 2020 •
edited
Loading

In the docs its really explicit that ElasticSearch should be populated with snake_case properties, but that gave me issues with normalizing nested objects. When I populated ElasticSearch with camelCase properties, serialization was Ok.

@dunglas dunglas requested a review from meyerbaptiste January 20, 2021 08:06
@soyuka
Copy link
Member

soyuka commented Jan 22, 2021

I stumbled upon this as well. Thanks for the edit.

@soyuka soyuka merged commit 398eaaa into api-platform:2.5 Jan 22, 2021
dunglas added a commit that referenced this pull request Jan 24, 2021
@dunglas
Merge branch '2.5'
9ef4e12 
* 2.5:
  update default branch references
  update supported branches
  Update operations.md (#1253)
  Update elasticsearch.md (#1204)
  Use int  type instead of integer deprecated type (#1252)
@meyerbaptiste
Copy link
Member

meyerbaptiste commented Jan 26, 2021 •
edited
Loading

This seems strange to me. Do you have an example/a snippet? It looks more like a bug to me.
IIRC, the best practice is snake case for ES and if so, we should follow it.

@meyerbaptiste meyerbaptiste mentioned this pull request Feb 11, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meyerbaptiste meyerbaptiste Awaiting requested review from meyerbaptiste

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@darthf1 @soyuka @meyerbaptiste