-
Notifications
You must be signed in to change notification settings - Fork 197
Fix Data Member Attribute json Serialized Model Casing Applied[#212] #263
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
Signed-off-by: choipureum <[email protected]>
Signed-off-by: choipureum <[email protected]>
Signed-off-by: poo <[email protected]>
Signed-off-by: poo <[email protected]>
Signed-off-by: poo <[email protected]>
Signed-off-by: poo <[email protected]>
Signed-off-by: poo <[email protected]>
Signed-off-by: poo <[email protected]>
Signed-off-by: choipureum <[email protected]>
Signed-off-by: choipureum <[email protected]>
Signed-off-by: choipureum <[email protected]>
Signed-off-by: poo <[email protected]>
Signed-off-by: poo <[email protected]>
Signed-off-by: poo <[email protected]>
Signed-off-by: poo <[email protected]>
Signed-off-by: poo <[email protected]>
Signed-off-by: ChoiPuReum <[email protected]>
Signed-off-by: ChoiPuReum <[email protected]>
Signed-off-by: ChoiPuReum <[email protected]>
Signed-off-by: poo <[email protected]>
Signed-off-by: poo <[email protected]>
justinyoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update! I've left some comments, mostly for the test cases.
|
|
||
| namespace Microsoft.Azure.WebJobs.Extensions.OpenApi.TestApp.Models | ||
| { | ||
| public class DataMemberObjectModel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we create another class/file for JsonPropertyObjectModel for another test?
| { | ||
| public class DataMemberObjectModel | ||
| { | ||
| [DataMember(Name = "DataMemberValue1")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're testing the property naming, can we set these property names in some weird ways like DAtAmeMBervALue1 so that it's rendered as expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried a variety of things.Thank you always for the reviews! 😊
|
|
||
| namespace Microsoft.Azure.WebJobs.Extensions.OpenApi.TestApp | ||
| { | ||
| public static class Get_ApplicationJson_DataMemberObject_HttpTrigger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we create another one for Get_ApplicationJson_JsonPropertyObject_HttpTrigger?
| { | ||
| [TestClass] | ||
| [TestCategory(Constants.TestCategory)] | ||
| public class Get_ApplicationJson_DataMember_Tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we create another test class, Get_ApplicationJson_JsonProperty_Tests?
justinyoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! LGTM
related issue: #212
This PR uses the name by
DataMemberAttributeas json serialization namingstrategy likeJsonProperty.