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

Upgrade Go to 1.22 #64

Merged
merged 2 commits into from
Jul 31, 2024
Merged

Upgrade Go to 1.22 #64

merged 2 commits into from
Jul 31, 2024

Conversation

zegerius
Copy link
Contributor

@zegerius zegerius commented Jul 30, 2024

Description of changes:

This PR:

This PR does not introduce any breaking changes.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@zegerius
Copy link
Contributor Author

Hi @knqyf263, as we use this tool in several CI flows I wanted to see if I can contribute some of my time to improve it. I noticed cob is still using Go 1.13. Hope you appreciate the contribution here and in #65.

@knqyf263
Copy link
Owner

Thanks for your contribution.

These functions were incorporated into the standard library's errors package in Go 1.13: - Is - As - Unwrap

xerrors.Errorf is not deprecated.

@zegerius
Copy link
Contributor Author

xerrors.Errorf is not deprecated.

Rephrased :)

@knqyf263
Copy link
Owner

I mean it's needed for stack trace.

@zegerius
Copy link
Contributor Author

If I understand correctly you want to use the stack backtrace that xerrors provides? I see the Go team removed the deprecation notice. I guess I can drop that change if you prefer. Wonder if it really adds any value - there's not that much special that can go wrong here.

@knqyf263
Copy link
Owner

Wonder if it really adds any value - there's not that much special that can go wrong here.

I'm curious why you're wondering. There are many cases where it fails and stack trace can help.

@zegerius
Copy link
Contributor Author

zegerius commented Jul 30, 2024

In this project the scope is so limited - it is running two commands and compare results. Good error handling and logging should IMO negate the introduction of a new dependency to inspect a stack.

But, I can tell you are convinced it adds value, so I'll remove the change from the PR

@knqyf263
Copy link
Owner

In this project the scope is so limited - it is running two commands and compare results.

I don't mind if you want to delete it, but it is not because it is deprecated and it should be made clear that there is a price to pay for losing stack traces. In other words, it is better to do it in another PR if you want.

Copy link
Owner

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

Thanks

@knqyf263 knqyf263 merged commit 640652d into knqyf263:master Jul 31, 2024
3 checks passed
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

Successfully merging this pull request may close these issues.

2 participants