-
Notifications
You must be signed in to change notification settings - Fork 40
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
refactor: consolidate fusion driver #208
Conversation
2411066
to
fcbd57d
Compare
ba56970
to
e589d5e
Compare
e589d5e
to
1de78ff
Compare
437bd61
to
bc81082
Compare
@lbajolet-hashicorp - this one is good to go now! |
0aea06f
to
79c9c26
Compare
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.
Left a couple comments on regexp management, but aside from that LGTM!
Pre-approving
79c9c26
to
018e73a
Compare
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.
Left one last comment since there's one regexp that isn't compiled outside the code, I'll let you answer my comment, if there's a good reason to have it compiled in the CheckOvfToolVersion
function we can roll with it
- Consolidates `Fusion5Driver` and `Fusion6Driver` to `FusionDriver` within `driver_fusion.go`. - Added constants to simplify functions. - Added functions to remove repedative path joins. Signed-off-by: Ryan Johnson <ryan@tenthirtyam.org>
018e73a
to
08cad59
Compare
LGTM, thanks for the reroll @tenthirtyam! Merging as soon as tests go green |
Description
Consolidates
Fusion5Driver
andFusion6Driver
toFusionDriver
withindriver_fusion.go
.Note: The minimum version should be set to VMware Fusion 13 per the Broadcom Product Lifecycle. However, we could hold this change, leave it as 6, and update all minimum versions simultaneously.
Testing
✅ Basic: PASS 🚀
Test Results
✅ End-to-End: PASS 🚀
Test Results