Merged
Conversation
Since the `packer fmt` command does not return a non-zero exit code if file are modified and the `packer validate` command has its own output if a template fails validation, it does not make sense to add more to the output of either command. This is especially true since the `packer fmt` command will output modified files and the `packer validate` command will mention the failing files in its output.
Wrap the core commands for each hook in command blocks and use the & operator to run them in the background (through forking). If any command fails the hook will exit with a non-zero exit code.
Break out the `packer validate` call so we can: - Disable errexit for _just_ the `packer validate` call so every path has an attempt at validation. - Get the output of the `packer validate` call so that if there is a non-zero exit code we can output that along with the path being validated.
mcdonnnj
added
bug
jsf9k
approved these changes
Dec 17, 2024
Member
jsf9k
left a comment
jsf9k
left a comment
There was a problem hiding this comment.
Approved, but please note my one question.
dv4harr10
approved these changes
Dec 17, 2024
This was referenced Dec 17, 2024
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
4 participants
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.
🗣 Description
This pull request changes both the
packer_fmtandpacker_validatehooks to run against each provided path in parallel using command blocks and the&operator. It also removes the custom output for thepacker_fmthook becausepacker fmtdoes not return a non-zero exit code if it formats files. The output for thepacker_validatehook is updated to correctly work for each path that is provided.💭 Motivation and context
This will help performance in more complex scenarios such as monorepos with a number of templates and/or files in a template. It also fixes the
packer_validatehook to correctly provide output for every path it is run against if any one or more of those paths were to fail validation. Incidentally this obviates the need for #53 because running core hook functionality in command blocks results in a forked process that does not change the current directory of the parent hook process.🧪 Testing
Automated tests pass. I tested this against scenarios that included a single template and multiple templates to ensure expected output and failure cases.
✅ Pre-approval checklist