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

move all static error messages to errors.go #310

merged 1 commit into from Feb 17, 2022


Copy link

@uturunku1 uturunku1 commented Feb 15, 2022


This is PR is grouping all instances of errors.New() into a single place.
go-tfe has a file errors.go where we have nicely organized and classified error messages. There are many other places in the repo where we are instantiating errors outside of this file. For the sake of consistency let's keep those errors that are static in errors.go along with other errors that live there.

Besides improving on consistency, another two little issues we are solving with this refactoring are:

  1. There were a bunch of errors.New() outside of errors.go that were creating the same error message unnecessarily.
  2. There were two errors created in errors.go that were going unused. They were probably added by the contributor intentionally, but then contributor forgot to use their own error. It happens.

Testing plan

External links

Output from tests (HashiCorp employees only)

$ TFE_ADDRESS="https://example" TFE_TOKEN="example" TF_ACC="1" go test ./... -v -tags=integration -run TestFunctionsAffectedByChange


@uturunku1 uturunku1 marked this pull request as ready for review Feb 17, 2022
@uturunku1 uturunku1 requested a review from a team as a code owner Feb 17, 2022
Copy link

@sebasslash sebasslash left a comment

This initial effort is awesome Luces! We'll know if we missed any when we introduce the goconst linter into our pipeline.

@uturunku1 uturunku1 merged commit 8ea1cc3 into releases-1.0.x Feb 17, 2022
4 checks passed
@uturunku1 uturunku1 deleted the group-errors branch Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet

Successfully merging this pull request may close these issues.

None yet

2 participants