-
Notifications
You must be signed in to change notification settings - Fork 6k
Refactor, improve the WebAPIClient tutorial page to use GetFromJsonAsync
instead of DeserializeAsync
#48202
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
Refactor, improve the WebAPIClient tutorial page to use GetFromJsonAsync
instead of DeserializeAsync
#48202
Conversation
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.
This looks great @BartoszKlonowski
I had a few comments, and then it's ready to merge.
This reverts commit bb708ac.
This is mentioned in two places - with the C# naming convention, where it's worth to mention why we keep on having uppercase, and in the Repository class defining, where we would normally had to make a conversion, but now we don't because we have case-insensitive method as a tool.
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.
This is great @BartoszKlonowski
I'll as soon as the build finishes.
This pull request closes #47386.
It uses the
System.Net.Http.Json.HttpClientJsonExtensions.GetFromJsonAsync
extension method to simplify the fetching and deserialization process in the tutorial to teach good practices from the beginning and use the latest available features.To achieve the full compatibility with what was there in the tutorial and the new method usage, this PR:
ProcessRepositoriesAsync
function to make use of theGetFromJsonAsync
method,Repository
class so that it doesn't have to use the complicatedJsonPropertyName
ofSystem.Text.Json.Serialization
, but puts all the class fields that we want to deserialize right away into the class fields,Write
toWriteLine
, so that at the beginning, when displaying only the repo name in the console we display it line by line, instead of all together (to slightly improve the readability).I know this displays correctly already in the final page in docs, but it just confused me at first, so wanted to align that with what is displayed in page.
Note
This PR should also have the PR to samples repo merged: dotnet/samples#7067, to make sure that the full, final, project matches the tutorial implementation explained.
Internal previews