Skip to content

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

Merged
ashpatil-msft merged 2 commits into
microsoft:masterfrom
ashpatil-msft:user/ashpatil/Bug
Mar 26, 2021
Merged

Rest endpoint helper and json object field check - Bug fix#821
ashpatil-msft merged 2 commits into
microsoft:masterfrom
ashpatil-msft:user/ashpatil/Bug

Conversation

@ashpatil-msft

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

Copy link
Copy Markdown
Contributor

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

@JohnMcPMS JohnMcPMS left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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