boxcutter / windows

Virtual machine templates for Windows written in legacy JSON and Batch Scripting/JScript
Apache License 2.0
753 stars 266 forks source link

Standardization of the order of the fields for the builders in all the templates #221

Open arizvisa opened 4 years ago

arizvisa commented 4 years ago

The order of the fields in the builders for some of the templates are somewhat haphazard. This is just the natural order of things as the schema changes over time.

This isn't really too big of a deal, but if more than 1 person submits a PR that tampers with all of the templates (which could be common), PRs will look a lot larger than they should to git/patch and so the submitter will be forced to re-base a lot more often.

This also affects users that apply their own patches to arbitrary commits in the repo because in some cases patch in fuzzy-mode has nothing to consistently anchor upon.

arizvisa commented 4 years ago

PR #220 currently re-organizes the ssh_username and ssh_password fields so they're anchored near the end between ssh_timeout and vm_name similar to the winrm_* parameters in the standard templates.

I don't plan on standardizing/documenting this other than maybe ensuring that the builders begin with their type and keeping cpus/cores/memory/etc. anchored near the vm_name since those shouldn't really change due to being controlled by user-variables.

arizvisa commented 4 years ago

Commit d81f5f83a7dac76dcf2bea5e556b191f089be922 does a little bit of re-ordering in order to test refactoring the scripts to perform downloads during provisioning instead of the building phase. This is partially related to issue #213 which has an issue while downloading wget.exe from the internet.

arizvisa commented 4 years ago

Commit 84747722c9c67bb6f9b54c4fb25632d1c5f28b9c re-orders the floppy_files field in all the builders by prefixing them with their respective Autounattend.xml script, and sorting the rest by name. This greatly simplifies comparison to distinguish what is different between all the templates.

Ftr, this was done with the following jq query:

.builders[] |= (.floppy_files = [.floppy_files[] | select(test("Autounattend.xml"))] + ([(.floppy_files[] | select(test("Autounattend.xml") | not ))] | sort))
arizvisa commented 4 years ago

As a result of sorting the floppy_files, a number of issues (#222, #224, #225) were unraveled. These correspond to the PRs (#223, #226, #227).

arizvisa commented 4 years ago

The "iso_checksum_type" which was formerly hardcoded to "sha1" is now a user-variable as per PR #236. This also positions the "iso_checksum_type" right below the "iso_url" in all the builders.

arizvisa commented 3 years ago

The shutdown_command user-variable was made consistent by PR #256.