-
Notifications
You must be signed in to change notification settings - Fork 62
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
Send arrays to ConvertTo-Json in the internal disk implementation #154
Send arrays to ConvertTo-Json in the internal disk implementation #154
Conversation
b2fdf07
to
700566d
Compare
I've also checked the other places in /unhold |
@@ -93,8 +93,8 @@ func (DiskAPI) ListDiskLocations() (map[uint32]shared.DiskLocation, error) { | |||
|
|||
m := make(map[uint32]shared.DiskLocation) | |||
for _, v := range getDisk { | |||
str := v["location"].(string) | |||
num := v["number"].(float64) | |||
str := v["Location"].(string) |
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.
why change location --> Location?
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 change is actually not needed and I can revert it back, it's because I also changed this line above:
cmd := fmt.Sprintf("ConvertTo-Json @(Get-Disk | select Number, Location)")
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 think I can skip making this change, I already run the integration tests on #155 and they're passing with the v1 API too
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jingxu97, mauriciopoppe The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind bug
What this PR does / why we need it:
Context #153, we're using
ConvertTo-Json @(...)
instead of pipesWhich issue(s) this PR fixes:
Fixes #153
Does this PR introduce a user-facing change?:
/hold
/cc @jingxu97 @ddebroy
We should merge #152 first