Skip to content
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

fix: use data_key from field if available to set property name #100

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jgroth
Copy link

@jgroth jgroth commented Dec 11, 2019

#88 introduced the ability to customize the name of the fields when serializing, which is great! However, Marshmallow already has a way to customize the name of fields when serializing through the data_key property. The JSONSchema should use the same property when serializing the schema to json instead of inventing its own thing.

@coveralls
Copy link

coveralls commented Dec 11, 2019

Coverage Status

Coverage remained the same at 99.149% when pulling 160c85a on Lundalogik:field_data_key into c032d1a on fuhrysteve:master.

@fuhrysteve fuhrysteve mentioned this pull request Feb 29, 2020
Marshmallow already has a way to customize the name of fields when serializing through the data_key
property. The JSONSchema should use the same property when serializing the schema to json instead of
inventing its own thing.
@jgroth
Copy link
Author

jgroth commented Feb 22, 2021

Ping @fuhrysteve
I have rebased this on master, it would be great to get this merged it's been a PR for a while now

@martijnthe
Copy link
Contributor

I’m only finding this PR now. I did more or less the same thing in #139 (comment)

@fuhrysteve
Copy link
Owner

thanks @martijnthe and @jgroth ! Apologies for not having seen this before now.

It looks like this changeset prefers data_key over metadata. Do either of you have a sense for which one follows the standard better? Seems like data_key should take precedence, however I'm not sure off the top of my head.

@jgroth
Copy link
Author

jgroth commented Mar 10, 2021

I removed the use if metadata in this PR since Marshmallow already had the the data_key concept and I thought it would only be confusing having two concepts doing the same thing. I you still want to keep it I think it is fine since this would be a breaking change, but I think supporting data_key is the most important part of this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants