Skip to content

refactor: consolidate player driver#231

Merged
lbajolet-hashicorp merged 1 commit intomainfrom
refactor/consolidate-player-driver
Oct 7, 2024
Merged

refactor: consolidate player driver#231
lbajolet-hashicorp merged 1 commit intomainfrom
refactor/consolidate-player-driver

Conversation

@tenthirtyam
Copy link
Copy Markdown
Collaborator

@tenthirtyam tenthirtyam commented Jul 18, 2024 •
edited
Loading

Description

Consolidates Player5Driver and Player6Driver to PlayerDriver within driver_player.go.

Testing

  • Basic
  • End-to-End
packer-plugin-vmware on  refactor/consolidate-player-driver
➜ go fmt ./...

packer-plugin-vmware on  refactor/consolidate-player-driver
➜ make generate                                            
2024/07/18 18:07:15 Copying "docs" to ".docs/"
2024/07/18 18:07:15 Replacing @include '...' calls in .docs/
Compiling MDX docs in '.docs' to Markdown in '.web-docs'...

packer-plugin-vmware on  refactor/consolidate-player-driver
➜ make build   

packer-plugin-vmware on  refactor/consolidate-player-driver
➜ make test    
?       github.com/hashicorp/packer-plugin-vmware       [no test files]
?       github.com/hashicorp/packer-plugin-vmware/version       [no test files]
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/common 6.704s
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/iso    2.066s
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/vmx    2.544s

packer-plugin-vmware on  refactor/consolidate-player-driver 
➜ make dev     
packer plugins install --path packer-plugin-vmware "github.com/hashicorp/vmware"
Successfully installed plugin github.com/hashicorp/vmware from /Users/ryan/Library/Mobile Documents/com~apple~CloudDocs/Code/Personal/packer-plugin-vmware/packer-plugin-vmware to /Users/ryan/.packer.d/plugins/github.com/hashicorp/vmware/packer-plugin-vmware_v1.0.12-dev_x5.0_darwin_arm64

@tenthirtyam tenthirtyam self-assigned this Jul 18, 2024
@tenthirtyam tenthirtyam mentioned this pull request Jul 18, 2024
Closed
@tenthirtyam tenthirtyam added this to the v1.1.1 milestone Jul 19, 2024
@tenthirtyam tenthirtyam force-pushed the refactor/consolidate-player-driver branch 2 times, most recently from 5a828c6 to 82753c2 Compare August 9, 2024 18:21
@tenthirtyam tenthirtyam marked this pull request as ready for review August 10, 2024 03:41
@tenthirtyam tenthirtyam requested a review from a team as a code owner August 10, 2024 03:41
@tenthirtyam tenthirtyam marked this pull request as draft August 26, 2024 15:58
@tenthirtyam tenthirtyam force-pushed the refactor/consolidate-player-driver branch 5 times, most recently from 8455cd4 to 56e5c45 Compare August 28, 2024 03:48
@tenthirtyam tenthirtyam force-pushed the refactor/consolidate-player-driver branch 5 times, most recently from bd0479a to 537d592 Compare September 10, 2024 02:01
@tenthirtyam tenthirtyam marked this pull request as ready for review September 10, 2024 02:28
@tenthirtyam
Copy link
Copy Markdown
Collaborator Author

tenthirtyam commented Sep 10, 2024

@lbajolet-hashicorp - this one is ready for initial review for the driver consolidation.

E2E testing is still pending but I will be testing it this week on both Windows (11) and Linux (Ubuntu 22.04) with Workstation Player 17.6.

Ryan

lbajolet-hashicorp
lbajolet-hashicorp approved these changes Sep 24, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

Left a couple nits, but overall this LGTM, thanks @tenthirtyam!

Pre-approving now, we can merge once you've had a chance to address my comments

@tenthirtyam tenthirtyam force-pushed the refactor/consolidate-player-driver branch from 537d592 to 5b336cb Compare September 24, 2024 20:01
@tenthirtyam tenthirtyam force-pushed the refactor/consolidate-player-driver branch from 5b336cb to 7229c55 Compare October 7, 2024 16:27
@tenthirtyam
refactor: consolidate player driver
ca282ef 
Consolidates `Player5Driver` and `Player6Driver` to `PlayerDriver` within `driver_player.go`.

Signed-off-by: Ryan Johnson <ryan@tenthirtyam.org>
@tenthirtyam tenthirtyam force-pushed the refactor/consolidate-player-driver branch from 7229c55 to ca282ef Compare October 7, 2024 16:54
lbajolet-hashicorp
lbajolet-hashicorp approved these changes Oct 7, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@lbajolet-hashicorp lbajolet-hashicorp 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 the rerolls @tenthirtyam

Merging now!

@lbajolet-hashicorp lbajolet-hashicorp merged commit 3b080bf into main Oct 7, 2024
@lbajolet-hashicorp lbajolet-hashicorp deleted the refactor/consolidate-player-driver branch October 7, 2024 17:09
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 31, 2026

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 Jan 31, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@lbajolet-hashicorp lbajolet-hashicorp lbajolet-hashicorp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tenthirtyam tenthirtyam

Labels

None yet

Projects

None yet

Milestone

v1.2.0

Development

Successfully merging this pull request may close these issues.

2 participants

@tenthirtyam @lbajolet-hashicorp