Skip to content

api: add sata controller#3458

Merged
dougm merged 1 commit intomainfrom
api/add-sata-controller
Jul 22, 2024
Merged

api: add sata controller#3458
dougm merged 1 commit intomainfrom
api/add-sata-controller

Conversation

@tenthirtyam
Copy link
Copy Markdown
Contributor

@tenthirtyam tenthirtyam commented May 31, 2024

Description

Adds support for creating SATA controllers.

Closes: #1219

Type of change

Please mark options that are relevant:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to
    not work as expected)
  • This change requires a documentation update
  • Build related change

How Has This Been Tested?

  • Local test: TestCreateSATAController
Running tool: /usr/local/bin/go test -timeout 30s -run ^TestCreateSATAController$ github.com/vmware/govmomi/object

ok  	github.com/vmware/govmomi/object	(cached)

Checklist:

  • My code follows the CONTRIBUTION guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged

@tenthirtyam tenthirtyam self-assigned this May 31, 2024
@tenthirtyam tenthirtyam mentioned this pull request May 31, 2024
Closed
3 tasks
dougm
dougm previously approved these changes Jun 1, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@dougm dougm 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 @tenthirtyam !

@tenthirtyam tenthirtyam requested a review from akutz June 3, 2024 16:26
akutz
akutz previously requested changes Jun 3, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@akutz akutz left a comment

Choose a reason for hiding this comment

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

Did you account for VirtualAHCIController?

@tenthirtyam tenthirtyam force-pushed the api/add-sata-controller branch from 052cc32 to 7593d1f Compare June 14, 2024 16:10
@tenthirtyam tenthirtyam marked this pull request as draft June 16, 2024 02:55
@tenthirtyam tenthirtyam force-pushed the api/add-sata-controller branch from 7593d1f to 57aea68 Compare July 8, 2024 22:06
@tenthirtyam tenthirtyam requested review from akutz and dougm July 8, 2024 22:22
@tenthirtyam tenthirtyam marked this pull request as ready for review July 8, 2024 22:23
@tenthirtyam
Copy link
Copy Markdown
Contributor Author

tenthirtyam commented Jul 8, 2024

I think that I've addressed your concerns, but let me know otherwise.

I still want to do a round of additional tests before merge.

@tenthirtyam tenthirtyam mentioned this pull request Jul 9, 2024
Merged
dougm
dougm reviewed Jul 19, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@dougm dougm left a comment

Choose a reason for hiding this comment

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

A few things I didn't notice on the 1st pass, esp. after having tried from the UI

@tenthirtyam
api: add sata controller
2e77836 
Adds support for creating SATA controllers.

Signed-off-by: Ryan Johnson <ryan.johnson@broadcom.com>
@tenthirtyam tenthirtyam force-pushed the api/add-sata-controller branch from 57aea68 to 2e77836 Compare July 19, 2024 20:53
@tenthirtyam
Copy link
Copy Markdown
Contributor Author

tenthirtyam commented Jul 19, 2024

A few things I didn't notice on the 1st pass, esp. after having tried from the UI

Thanks, Doug! I've addressed your comments - appreciate your help!

@tenthirtyam tenthirtyam requested a review from dougm July 19, 2024 20:56
dougm
dougm approved these changes Jul 19, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@dougm dougm 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 @tenthirtyam

@tenthirtyam tenthirtyam dismissed akutz’s stale review July 19, 2024 21:34

Addressed the changes requested.

@tenthirtyam tenthirtyam requested review from akutz and removed request for akutz July 19, 2024 21:35
@dougm dougm merged commit f05efc4 into main Jul 22, 2024
@dougm dougm deleted the api/add-sata-controller branch July 22, 2024 17:44
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 17, 2025

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@dougm dougm dougm approved these changes

@akutz akutz Awaiting requested review from akutz

Assignees

@tenthirtyam tenthirtyam

Labels

area/govc cla-not-required kind/enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add support for SATA controller

4 participants

@tenthirtyam @dougm @akutz @vmwclabot