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

Fix 'OpenVirtualDiskParameters' BOOL fields #226

Merged
merged 2 commits into from
Feb 15, 2022

Conversation

dcantah
Copy link
Contributor

@dcantah dcantah commented Dec 1, 2021

While reworking the vhd package I'd mistakingly replaced some of the fields
in the OpenVersion2 structure with the incorrect types. They're defined as
Windows BOOLS which is just a type alias for an int, and I'd put their type as
go bools.

As the type is already passed into some functions, to avoid a breaking change just
convert the incorrect type to a new structure with the correct definition internally.
Truthfully this turns out a bit better as supplying a bool makes more sense and is
more go friendly.

Signed-off-by: Daniel Canter dcanter@microsoft.com

@dcantah dcantah requested a review from a team as a code owner December 1, 2021 03:15
vhd/vhd.go Show resolved Hide resolved
vhd/vhd.go Show resolved Hide resolved
Copy link
Contributor

@ambarve ambarve left a comment

Choose a reason for hiding this comment

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

LGTM

vhd/vhd.go Outdated Show resolved Hide resolved
While reworking the vhd package I'd mistakingly replaced some of the fields
in the OpenVersion2 structure with the incorrect types. They're defined as
Windows BOOLS which is just a type alias for an int, and I'd put their type as
go bools.

As the type is already passed into some functions, to avoid a breaking change just
convert the incorrect type to a new structure with the correct definition internally.
Truthfully this turns out a bit better as supplying a bool makes more sense and is
more go friendly.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
Change openVersion2 comment

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@dcantah
Copy link
Contributor Author

dcantah commented Jan 6, 2022

@katiewasnothere PTAL

Copy link
Contributor

@katiewasnothere katiewasnothere left a comment

Choose a reason for hiding this comment

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

lgtm

@dcantah dcantah merged commit 7de02db into microsoft:master Feb 15, 2022
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.

3 participants