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

Rest endpoint helper and json object field check - Bug fix #821

Merged
merged 2 commits into from
Mar 26, 2021

Conversation

ashpatil-msft
Copy link
Contributor

@ashpatil-msft ashpatil-msft commented Mar 26, 2021

Fixed two bugs in the Rest source code:

  • Added check to format Rest API Uri when attempting to get Information from Rest source
  • Added check to see if field is present before getting value for some fields that got missed.

Manually tested and verified basic install and manifest deserialization.

Microsoft Reviewers: Open in CodeFlow

@ashpatil-msft ashpatil-msft requested a review from a team as a code owner March 26, 2021 07:04
@ashpatil-msft ashpatil-msft changed the title Rest endpoint helper and json object field check Rest endpoint helper and json object field check - Bug fix Mar 26, 2021
Copy link
Member

@JohnMcPMS JohnMcPMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we obviously need tests eventually, and I think we do need to do the http client injection for the unit tests so that we can easily test the corner cases.

namespace AppInstaller::Repository::Rest::Schema
{
// Rest source helper.
struct RestHelper
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have to put every function inside a class type.

@ashpatil-msft ashpatil-msft merged commit d25870a into microsoft:master Mar 26, 2021
sreadingMSFT added a commit to sreadingMSFT/winget-cli that referenced this pull request Mar 26, 2021
Rest endpoint helper and json object field check - Bug fix (microsoft#821)
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