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
Ignore existing package hashes for providers lock
command
#31389
Merged
Merged
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
9 changes: 9 additions & 0 deletions
9
internal/command/testdata/providers-lock/append/.terraform.lock.hcl
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Empty file.
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
terraform { | ||
required_providers { | ||
test = { | ||
source = "hashicorp/test" | ||
} | ||
} | ||
} |
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look like what our usual tools would generate, but I must admit I'm not entirely sure what usual tool is responsible for it, and since the checks all passed I guess it might be something we have in our local editors only rather than something enforced by our CI process. 😖
Our typical basic style here is to separate the standard library packages from the external ones, with the standard library ones first and then a blank line followed by the external ones, with each separate "block" of packages in lexical order. I'd suggest manually changing this to match that for now, and then separately perhaps we can figure out which of the tools we're missing from our CI checks to catch this automatically in the future.
For some files that have a lot of imports we do also sometimes split the Terraform-internal packages from other external packages, so there would then be three "blocks" of packages: standard library, external, and then Terraform-internal. However, that's a more subjective call that our tooling does not enforce, and this one doesn't seem like it has enough packages to warrant that amount of fussiness, so I'm mentioning it only in the interests of helping you to see the patterns in other files in future. 😀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "default" grouping for imports is whatever
goimports
creates automatically, which is usually bundled with the standard group of editor plugins. The CI checks only verify whatgo fmt
produces, which does not re-order imports on its own. It would be good to get that tool (or equivalent) working, since anyone else opening the file is going to rewrite it in their next commit.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! And yeah it looks like goimports isn't installed by Goland automatically: https://www.jetbrains.com/help/go/integration-with-go-tools.html#goimports
I have now configured it.
Brief sidebar into/shoutout for https://pre-commit.com/ as something I've used in the past to make sure CI/CD and pre-commit checks on engineers machines are consistent (works across IDEs/architectures, forces common versions for all tools, etc).