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

Update DTOs to use nullability #27

Merged
merged 1 commit into from Nov 7, 2019

Conversation

@MingweiSamuel
Copy link
Owner

MingweiSamuel commented Nov 7, 2019

Updates DTOs to use C# 8 ? nullability. Also sorts the fields by name.

Concerns: depends on newtonsoft / system.text.json respecting the nullability. Actual constructors don't enforce it.

@RyadaProductions Any thoughts? If you'd like to review larger changes

@RyadaProductions

This comment has been minimized.

Copy link
Contributor

RyadaProductions commented Nov 7, 2019

Looks good to me. As far as i know both Newtonsoft and System.Text.Json respect nullability from C#8.
Also since we are working on the DTO's how realistic would it be to create a different file for each DTO?

@MingweiSamuel MingweiSamuel merged commit 8874098 into master Nov 7, 2019
1 check passed
1 check passed
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
@MingweiSamuel

This comment has been minimized.

Copy link
Owner Author

MingweiSamuel commented Nov 7, 2019

It would be reasonable. Might be worth converting the "here's a template that outputs a bunch of DTOs in a namespace" to a "here's a JS function that outputs a single DTO class" that can be used in both RiotAPi and LCU

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.