Skip to content

chore: error technical debt#239

Merged
lbajolet-hashicorp merged 1 commit intomainfrom
chore/error-tech-debt
Aug 21, 2024
Merged

chore: error technical debt#239
lbajolet-hashicorp merged 1 commit intomainfrom
chore/error-tech-debt

Conversation

@tenthirtyam
Copy link
Copy Markdown
Collaborator

@tenthirtyam tenthirtyam commented Aug 14, 2024

Summary

  • Update errors that don't require formatting to use errors.New instead of fmt.Errorf to be more idiomatic.
  • Updates UI errors that need formatting to ui.Errorf instead of ui.Error(fmt.Sprintf(
  • Correct spelling mistakes in docs/comments.

Testing

packer-plugin-vmware on  chore/error-tech-debt [$!] via 🐹 v1.22.6 on 🐳 v26.1.3 (rancher-desktop) 
➜ go fmt ./...

packer-plugin-vmware on  chore/error-tech-debt [$!] via 🐹 v1.22.6 on 🐳 v26.1.3 (rancher-desktop) 
➜ make generate
2024/08/14 15:59:10 Copying "docs" to ".docs/"
2024/08/14 15:59:10 Replacing @include '...' calls in .docs/
Compiling MDX docs in '.docs' to Markdown in '.web-docs'...

packer-plugin-vmware on  chore/error-tech-debt [$!] via 🐹 v1.22.6 on 🐳 v26.1.3 (rancher-desktop) took 6.3s 
➜ make build

packer-plugin-vmware on  chore/error-tech-debt [$!] via 🐹 v1.22.6 on 🐳 v26.1.3 (rancher-desktop) took 6.1s 
➜ make test
?       github.com/hashicorp/packer-plugin-vmware       [no test files]
?       github.com/hashicorp/packer-plugin-vmware/version       [no test files]
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/common 8.199s
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/iso    4.605s
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/vmx    6.139s

packer-plugin-vmware on  chore/error-tech-debt [$!] via 🐹 v1.22.6 on 🐳 v26.1.3 (rancher-desktop) took 17.1s 
➜ make dev
packer plugins install --path packer-plugin-vmware "github.com/hashicorp/vmware"
Successfully installed plugin github.com/hashicorp/vmware from /Users/johnsonryan/Library/Mobile Documents/com~apple~CloudDocs/Code/Work/packer-plugin-vmware/packer-plugin-vmware to /Users/johnsonryan/.packer.d/plugins/github.com/hashicorp/vmware/packer-plugin-vmware_v1.1.1-dev_x5.0_darwin_amd64

@tenthirtyam tenthirtyam added this to the v1.1.1 milestone Aug 14, 2024
@tenthirtyam tenthirtyam self-assigned this Aug 14, 2024
@tenthirtyam tenthirtyam requested a review from a team as a code owner August 14, 2024 20:00
@tenthirtyam tenthirtyam force-pushed the chore/error-tech-debt branch from 21c4a97 to ff9069e Compare August 14, 2024 20:00
@tenthirtyam tenthirtyam changed the title chore: error technical debt. chore: error technical debt Aug 14, 2024
@tenthirtyam tenthirtyam force-pushed the chore/error-tech-debt branch 4 times, most recently from 774f2c8 to c3da281 Compare August 15, 2024 01:58
lbajolet-hashicorp
lbajolet-hashicorp approved these changes Aug 19, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

Overall this looks good to me!

I left a few comments on some parts of the code, mostly the one thing that I think is superfluous is replacing %s by %v for error interpolation in strings, they're essentially the same thing from what I understand, so this may be dropped from this PR, unless there's a compelling reason to keep them.

Besides those, LGTM! Pre-approving so it's not a blocker later.

@tenthirtyam
Copy link
Copy Markdown
Collaborator Author

tenthirtyam commented Aug 19, 2024

Sure, I'll revert thr #v to #s.

@tenthirtyam tenthirtyam force-pushed the chore/error-tech-debt branch 5 times, most recently from 04aaa64 to cc75026 Compare August 19, 2024 22:21
@tenthirtyam
chore: error technical debt
ce1d2d5 
- Update errors that don't require formatting to use `errors.New` instead of  `fmt.Errorf` to be more idiomatic.
- Updates UI errors that need formatting to `ui.Errorf` instead of `ui.Error(fmt.Sprintf(`
- Correct spelling mistakes in docs/comments.

Signed-off-by: Ryan Johnson <ryan.johnson@broadcom.com>
@tenthirtyam tenthirtyam force-pushed the chore/error-tech-debt branch from cc75026 to ce1d2d5 Compare August 19, 2024 22:28
lbajolet-hashicorp
lbajolet-hashicorp approved these changes Aug 21, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

LGTM!

@lbajolet-hashicorp lbajolet-hashicorp merged commit 424cf2b into main Aug 21, 2024
@lbajolet-hashicorp lbajolet-hashicorp deleted the chore/error-tech-debt branch August 21, 2024 20:23
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 31, 2026

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 31, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@lbajolet-hashicorp lbajolet-hashicorp lbajolet-hashicorp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tenthirtyam tenthirtyam

Labels

chore Chore

Projects

None yet

Milestone

v1.2.0

Development

Successfully merging this pull request may close these issues.

2 participants

@tenthirtyam @lbajolet-hashicorp