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.

Sign up for GitHub

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

Jump to bottom

Add HostName field to QueryResultVMRecordType in order to have information about Hypervisor #503

Merged
merged 12 commits into from
Aug 31, 2022
Merged

Add HostName field to QueryResultVMRecordType in order to have information about Hypervisor #503

merged 12 commits into from
Aug 31, 2022

Conversation

thomsmoreau
Copy link
Contributor

@thomsmoreau thomsmoreau commented Aug 26, 2022 •
edited
Loading

Description

Related issue: #486 (comment)

  • This PR is made in order to add the info about the hostname/hypervisor of the VM, this change is needed since having this information helps us to identify cluster in our infrastructure.

thomas.moreau added 2 commits August 26, 2022 15:36
feat(go): adding in types.go the fiels Hypervisor from xml body hostN…
91ce003 
…ame in order to complete the information collected via the xml body of the request
Merge branch 'main' of https://github.com/vmware/go-vcloud-director i…
b7eee07 
…nto hostNameVm
@vmwclabot
Copy link
Member

@thomsmoreau, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

@vmwclabot
Copy link
Member

@thomsmoreau, we have received your signed contributor license agreement. It will be reviewed by VMware shortly. Another comment will be added to the pull request to notify you when the merge can proceed.

Didainius
Didainius reviewed Aug 29, 2022
View reviewed changes
Copy link
Collaborator

@Didainius Didainius left a comment

Choose a reason for hiding this comment

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

Hello,
Thanks for PR - I think it has a few things to remove as per your own comments.
Could you briefly give an explanation of what this PR adds.

Also - a changelog record in https://github.com/vmware/go-vcloud-director/tree/main/.changes/v2.17.0 (see other posts for inspiration)

Didainius
Didainius reviewed Aug 29, 2022
View reviewed changes
@thomsmoreau
Copy link
Contributor Author

Cleared useless comments and adding changes in v2.17.0

Didainius
Didainius reviewed Aug 29, 2022
View reviewed changes
adambarreiro
adambarreiro reviewed Aug 29, 2022
View reviewed changes
@vmwclabot
Copy link
Member

@thomsmoreau, VMware has approved your signed contributor license agreement.

Didainius
Didainius approved these changes Aug 29, 2022
View reviewed changes
Copy link
Collaborator

@Didainius Didainius left a comment

Choose a reason for hiding this comment

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

Thank for these fixes. Its good now.

lvirbalas
lvirbalas requested changes Aug 30, 2022
View reviewed changes
@adambarreiro adambarreiro mentioned this pull request Aug 30, 2022
Merged
fix: changing Name of field 'Hypervisor' to 'HostName' in QueryResult…
5dd6d8f 
…VMRecordType struct in orderto match XML field name
@thomsmoreau thomsmoreau changed the title Host name vm Add HostName field to QueryResultVMRecordType in order to have information about Hypervisor Aug 30, 2022
@thomsmoreau @adambarreiro
Update .changes/v2.17.0/503-enhancement.md
c5f0429 
Co-authored-by: Adam Barreiro <adambarreiro@users.noreply.github.com>
lvirbalas
lvirbalas approved these changes Aug 31, 2022
View reviewed changes
Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

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

Thanks for contributing!

adambarreiro
adambarreiro approved these changes Aug 31, 2022
View reviewed changes
Copy link
Collaborator

@adambarreiro adambarreiro left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for contributing.

@Didainius Didainius merged commit cc66003 into vmware:main Aug 31, 2022
@adambarreiro adambarreiro mentioned this pull request Sep 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adambarreiro adambarreiro adambarreiro approved these changes

@lvirbalas lvirbalas lvirbalas approved these changes

@Didainius Didainius Didainius approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@thomsmoreau @vmwclabot @adambarreiro @lvirbalas @Didainius