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

Multiple var files #6

Merged
merged 6 commits into from Jan 14, 2021
Merged

Conversation

msurovcak
Copy link
Contributor

Hi,

I am using "hierarchical" structure of tfvars files to say DRY. This change allows using multiple files to support multiple files the same as Terraform does with -var-file.

I also added/fixed help and added timeout to lint.

Thanks for the tfvar tool, it makes my life easier.

Copy link
Owner

@shihanng shihanng left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! Overall looks good. I left a few comments. Will approve and merge after they are addressed.

Makefile Outdated
@@ -9,7 +9,7 @@ test: ## Run tests
go test -covermode atomic -coverprofile=profile.cov -race -v ./... -count=1

lint: ## Run GolangCI-Lint
docker run --rm -v $$(pwd):/app -w /app golangci/golangci-lint:latest golangci-lint run -v
docker run --rm -v $$(pwd):/app -w /app golangci/golangci-lint:latest golangci-lint run -v --timeout 120s
Copy link
Owner

Choose a reason for hiding this comment

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

Any reason why we need to specify timeout here? Did you face any issue running make lint?

cmd/cmd.go Outdated
@@ -52,7 +52,7 @@ variable definitions files e.g. terraform.tfvars[.json] *.auto.tfvars[.json]`)
rootCmd.PersistentFlags().Bool(flagNoDefault, false, "Do not use defined default values")
rootCmd.PersistentFlags().StringArray(flagVar, []string{}, `Set a variable in the generated definitions.
This flag can be set multiple times.`)
rootCmd.PersistentFlags().String(flagVarFile, "", `Set variables from a file.`)
rootCmd.PersistentFlags().StringArray(flagVarFile, []string{}, `Set variables from a file.`)
Copy link
Owner

Choose a reason for hiding this comment

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

How about the following? To be consistent with --var above.

Suggested change
rootCmd.PersistentFlags().StringArray(flagVarFile, []string{}, `Set variables from a file.`)
rootCmd.PersistentFlags().StringArray(flagVarFile, []string{}, `Set variables from a file.
This flag can be set multiple times.`)

@@ -0,0 +1 @@
image_id = "abc"
Copy link
Owner

Choose a reason for hiding this comment

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

Could you add a newline here? Then the ⛔ symbol should be gone 😄

@shihanng
Copy link
Owner

Added some commits to address minor points. Going to merge this. Thank you for the PR @msurovcak.

@shihanng shihanng self-requested a review January 14, 2021 13:21
@shihanng shihanng merged commit 3918bf1 into shihanng:master Jan 14, 2021
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.

None yet

2 participants