feat: Add non-blocking warning for Terraform provider repo naming convention #1506
Conversation
AshGodfrey
commented
Jul 9, 2025
AshGodfrey
commented
- Add TerraformNamingWarning type and validation logic
- Display styled warning when repository doesn't follow terraform-provider-{NAME} pattern
- Warning is non-blocking and includes reference to HashiCorp docs
- Add comprehensive unit and integration tests
- Add TerraformNamingWarning type and validation logic
- Display styled warning when repository doesn't follow terraform-provider-{NAME} pattern
- Warning is non-blocking and includes reference to HashiCorp docs
- Add comprehensive unit and integration tests
AshGodfrey
changed the title
AshGodfrey
changed the title
AshGodfrey
changed the title
AshGodfrey
changed the title
|
@bflad were you thinking this for this issue? |
| // IsTerraformNamingWarning checks if an error is a TerraformNamingWarning | ||
| func IsTerraformNamingWarning(err error) bool { | ||
| _, ok := err.(*TerraformNamingWarning) | ||
| return ok | ||
| } |
There was a problem hiding this comment.
Nit: https://pkg.go.dev/errors#As can be used to avoid defining this function, makes it safer in case the error is wrapped by other errors during refactoring, and makes accessing inner details easier.
e.g. instead of
if prompts.IsTerraformNamingWarning(err) {That logic can use something like this to quickly check the error type:
if errors.As(err, new(prompts.TerraformNamingWarning)) {If you actually wanted to reference the error type fields, likeRepoName, a variable can be used:
var terraformNamingWarningErr prompts.TerraformNamingWarning
if errorrs.As(err, &terraformNamingWarningErr) {
// now we have terraformNamingWarningErr.RepoNameNot necessary in this context since the error message already has RepoName in there, but just wanted to mention. 👍
| repoName := urlParts[len(urlParts)-1] | ||
| if err := checkTerraformRepositoryNaming(repoName); err != nil { | ||
| // Return the target with the warning attached | ||
| return target, err |
There was a problem hiding this comment.
Since this return is happening before target.Publishing is being set, technically its acting more like an error rather than a warning. I think we'll want to decide what's better or worse here:
- (Current) Not updating the workflow configuration.
- (Alternative) Updating the workflow configuration, then raising the warning.
I think we want the latter by moving this logic below target.Publishing =, since its possible for someone to rectify the issue outside the Speakeasy CLI (unless there's other steps necessary).
