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

[for go-getter v1] use %w instead of %s to wrap error #474

Open
Ericwww opened this issue Feb 21, 2024 · 3 comments
Open

[for go-getter v1] use %w instead of %s to wrap error #474

Ericwww opened this issue Feb 21, 2024 · 3 comments

Comments

@Ericwww
Copy link

Ericwww commented Feb 21, 2024

Hi all,

I am still working with go-getter v1.7.3.

I found that in this version, %s is used instead of %w when wrapping errors via fmt.Errorf.
As a result, callers of go-getter will not be able to handle errors gracefully through methods such as errors.As or errors.Unwrap.

If you think the above needs to be changed, I will take my effort to make a PR to fix this. 😃😃

@Ericwww Ericwww changed the title [v1] use %w instead of %s to wrap error [for go-getter v1] use %w instead of %s to wrap error Feb 21, 2024
@Ericwww
Copy link
Author

Ericwww commented Feb 22, 2024

Hi, @jbardin
Sorry to bother you.

I'm not sure if you are the maintainer of this repository.

If you had time, may you discuss this issue with core team and make a decision?

@crw
Copy link
Contributor

crw commented Mar 6, 2024

Thanks for this submission. We reviewed it today in triage. Although this is the correct action for modernizing error handing in go-getter, the go-getter project is not under active development and is essentially frozen. The requirements of the functionality that go-getter provides to terraform and other HashiCorp projects are met with the current implementation, and so barring a change in needed functionality, this project shall continue largely unchanged.

This is just to set expectations with regards to this pull request. I am happy to leave it open in case we decide to merge the functionality in the future. One thing to note, it seems you have committed the files with spaces changed to tabs, which creates a relatively noisy diff. If you intend to leave this PR open, you may want to correct that to make a future review easier to complete.

Thanks again for your interest and your submission!

@Ericwww
Copy link
Author

Ericwww commented Mar 6, 2024

@crw Thank you for your reply. I'd be happy to keep it open.

Some noisy change were probably due to go fmt tool or other format tool provided by IDE.

Now, I have fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants