Skip to content
This repository has been archived by the owner on Jan 6, 2023. It is now read-only.

add box_download_checksum support (rewrite) #93

Merged
merged 4 commits into from
May 12, 2021
Merged

add box_download_checksum support (rewrite) #93

merged 4 commits into from
May 12, 2021

Conversation

konstruktoid
Copy link
Contributor

#90 rewrite

Signed-off-by: Thomas Sjögren konstruktoid@users.noreply.github.com

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
@apatard
Copy link
Member

apatard commented Dec 14, 2020

Looks a good idea (even if I would prefer GPG to verify things but it's not possible since Vagrant doesn't support that.). I've only comments on the way things have been implemented. Please address them.

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
Copy link
Member

@apatard apatard left a comment

Choose a reason for hiding this comment

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

Even if a better commit message would be nice, I'm not sure there's a defined policy about it, so I'm fine with it.
The settings may be already be set with other module options, as for box_version or box_url but some options like this possibly deserve an easy way to set them.

@konstruktoid konstruktoid changed the title add box_download_checksum support, part 2 add box_download_checksum support (rewrite) Dec 16, 2020
@konstruktoid
Copy link
Contributor Author

You're referring to 73a6e51? Yeah, that was a poor one. Updated the PR title.

What do you mean by The settings may be already be set with other module options? The standard Vagrantfile options are https://www.vagrantup.com/docs/vagrantfile/machine_settings#config-vm-box_download_checksum and https://www.vagrantup.com/docs/vagrantfile/machine_settings#config-vm-box_download_checksum_type. I don't really see any reason to deviate from those.

@apatard
Copy link
Member

apatard commented Dec 16, 2020

What do you mean by The settings may be already be set with other module options? The standard Vagrantfile options are https://www.vagrantup.com/docs/vagrantfile/machine_settings#config-vm-box_download_checksum and https://www.vagrantup.com/docs/vagrantfile/machine_settings#config-vm-box_download_checksum_type. I don't really see any reason to deviate from those.

oh, I was unclear. Sorry, I was talking of using config_options or instance_raw_config_args in the molecule.yml file.

@konstruktoid
Copy link
Contributor Author

I agree, but that discussion seems off topic in this PR. New issue/discussion?

@apatard
Copy link
Member

apatard commented Dec 16, 2020

I agree, but that discussion seems off topic in this PR. New issue/discussion?

Oh, no need for a new issue/discussion. I only wanted to explain why I was approving the PR.

@ssbarnea ssbarnea added the enhancement New feature or request label May 12, 2021
@ssbarnea ssbarnea merged commit 4932ce6 into ansible-community:master May 12, 2021
@konstruktoid konstruktoid deleted the boxchecksum branch May 12, 2021 13:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants