require release type in utility scripts - #132
Open
amcmanus wants to merge 2 commits into
Open
Conversation
arsalansufi
reviewed
Aug 18, 2026
arsalansufi
left a comment
Contributor
There was a problem hiding this comment.
Super delayed review but better late than never
| #!/bin/bash | ||
|
|
||
| vxiso_tarball=$1 | ||
| RELEASE_TYPE=${2:-"field"} |
Contributor
There was a problem hiding this comment.
Hm so I don't think that we should default the value to field. Because then, if you forget to specify the second param but are extracting an admin vx-iso image, you're gonna persist it at the wrong location silently. I think that the second param should always be explicitly specified. I'm happy with this approach if we drop the default/fallback value.
amcmanus
marked this pull request as ready for review
August 31, 2026 15:13
Contributor
Author
|
This will close votingworks/vxsuite#7197 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an initial solution to a feature request that would use release type asset directories for vx-iso rather than using a shared directory that is overwritten each time. This enables installing different release types without having to extract the image each time you switch between release types.
While this adds some level of convenience, it adds to the complexity of the local user environment when managing different releases. For example, a user could extract a
fieldrelease with theadminrelease type. That would put a validfieldrelease under theadminassets directory structure. If an olderfieldrelease were still present, it's possible that release could then be installed instead of the most recently extracted. Similarly, a user could intend to install anadminrelease, not realizing afieldrelease was most recently extracted there by mistake.@arsalansufi, I don't want to over-rotate on this, but I'd be interested in whether you think there's value in taking the time and effort to consider something like embedding the release type in a release to help with some of this basic validation? There's probably a follow-up question on whether we should add another check for the release version for many of the same reasons.