Skip to content

chore: replace reflect.DeepEqual#196

Merged
lbajolet-hashicorp merged 1 commit intomainfrom
chore/replace-reflect.DeepEqual
May 28, 2024
Merged

chore: replace reflect.DeepEqual#196
lbajolet-hashicorp merged 1 commit intomainfrom
chore/replace-reflect.DeepEqual

Conversation

@tenthirtyam
Copy link
Copy Markdown
Collaborator

Summary

Replaces the use of reflect.DeepEqual with errors.Is for error comparison.

Testing

packer-plugin-vmware on  chore/replace-reflect.DeepEqual [!] via 🐹 v1.22.3 make generate
2024/05/25 00:44:22 Copying "docs" to ".docs/"
2024/05/25 00:44:22 Replacing @include '...' calls in .docs/
Compiling MDX docs in '.docs' to Markdown in '.web-docs'...

packer-plugin-vmware on  chore/replace-reflect.DeepEqual [!] via 🐹 v1.22.3 took 4.7s make build   

packer-plugin-vmware on  chore/replace-reflect.DeepEqual [!] via 🐹 v1.22.3 took 3.0s 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 6.592s
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/iso    1.839s
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/vmx    2.230s

@tenthirtyam tenthirtyam added this to the v1.0.12 milestone May 25, 2024
@tenthirtyam tenthirtyam self-assigned this May 25, 2024
@tenthirtyam tenthirtyam requested a review from a team as a code owner May 25, 2024 04:46
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.

Left one quick comment on the usage of an else, and this looks good to me, however may I ask why we want to get rid of those DeepEqual statements? Have they become problematic in some way?

Replaces the use of `reflect.DeepEqual` with `errors.Is` for error comparison.

Signed-off-by: Ryan Johnson <ryan@tenthirtyam.org>
@tenthirtyam tenthirtyam force-pushed the chore/replace-reflect.DeepEqual branch from 214f2bd to d544221 Compare May 28, 2024 16:36
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.

Reroll looks good to me!
I still wonder why we want to get rid of those reflect.DeepEqual though, not that it's a problem to remove them but I can't help but ask what's the motivation behind the change :)

Merging this

@lbajolet-hashicorp lbajolet-hashicorp merged commit 37e8670 into main May 28, 2024
@lbajolet-hashicorp lbajolet-hashicorp deleted the chore/replace-reflect.DeepEqual branch May 28, 2024 17:59
@vmware vmware locked as resolved and limited conversation to collaborators Jun 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

chore Chore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants