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

Remove iso checksum type and fix tests #48

Conversation

somerandomqaguy
Copy link

Remove iso_checksum_type checks

This aligns the Xenserver plugin to being a bit more inline with what Packer > 1.6.0 is expecting, since packer now simply ignores the iso_checksum_type (it's supposed to error out but that code path isn't working right now because we don't set PluginType in the configs. The unit tests have been altered to reflect this reality.

Note that this isn't a comprehensive change; the config still has the inert ISOChecksumType, and there's probably a laundry list of other things that needs to be looked at, For now though, we have working unit tests again.

Documentation has been updated to reflect this new change as well.

Looks like the consts got moved into common
Solves TestBuilderPrepare_InvalidKey failing, since we weren't capturing
it's errors before.
This aligns the Xenserver plugin to being a bit more inline with what
Packer > 1.6.0 is expecting, since packer now simply ignores the
iso_checksum_type (it's supposed to error out but that code path isn't
working right now because we don't set PluginType in the configs. The
unit tests have been altered to reflect this reality.

Note that this isn't a comprehensive change; the config still has the
inert ISOChecksumType, and there's probably a laundry list of other
things that needs to be looked at, For now though, we have working
unit tests again.

Documentation from SDK has been aligned for iso_checksum
@ddelnano
Copy link
Owner

Greatly appreciate you taking the time to improve the packer plugin! I've been behind on reviewing the recent PRs for this project, but I plan to look at this within the next week or two!

@ddelnano
Copy link
Owner

ddelnano commented Apr 7, 2023

@somerandomqaguy I'm extremely sorry this took so long to follow up on. I've added a few changes on top of your original change to fully remove iso_checksum_type entirely in #60.

I really appreciate your contribution and I should be more responsive for this repo in the future now that I'm more actively using it.

@ddelnano ddelnano closed this Apr 7, 2023
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.

2 participants