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

Added JSONName attribute to specify a custom name to use when importing and exporting #24

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

Conversation

TJHeuvel
Copy link

@TJHeuvel TJHeuvel commented Jul 8, 2014

Whenever a field has the JSONName property it will use that name, instead of the fieldname. This allows you to slightly deviate from the incoming or outgoing JSON.

@lbv
Copy link
Collaborator

lbv commented Aug 17, 2014

Hi,

Thank you for the contribution. This seems interesting and probably something I'd like to add. A few comments:

  • Regarding the JsonIgnore commit, it seems like it's the same issue from pull request Add JsonIgnore Attribute #7. I'd invite you to share your point of view there, and leave this pull request only for the JsonName issue (please remove the commit TJHeuvel@f503075 if that's the case).
  • Related to the previous point, I'd like to review the changes, but as it stands right now it's a mess. I know the original coding style of this library is terrible (I was young, more naive, and just as ignorant as today, which is to say a lot), but "fixing" things like whitespaces along with real code changes is not a very good practice if you want those changes merged into the original codebase.
  • Before merging this type of changes (adding new attributes and the like), I'd like to check other projects first and see if they have similar features, mainly because I don't want to use different names for the same functionality. If other JSON libraries implement something similar, and they use the JsonName attribute, that's great; otherwise I'd like to choose the name for that symbol carefully based on what introduces the least disruption in codebases that currently use other libraries. If you can help me with this task of reviewing what other .Net libraries do, I'd appreciate it very much :).

Thanks.

@devlead devlead changed the base branch from master to develop December 23, 2017 23:45
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.

2 participants