Skip to content

🌱 Replace errors pkg with stdlib #2439

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

🌱 Replace errors pkg with stdlib #2439

wants to merge 2 commits into from

Conversation

tylerauerbeck
Copy link
Contributor

What this PR does / why we need it:

This PR updates the usage of github.com/pkg/errors to instead utilize the errors package. A majority of cases just involved moving from errors.Wrapf to fmt.Errorf("some error: %w", err to match the existing formatting. This is needed as the github.com/pkg/errors has been deprecated for some time.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2410

@metal3-io-bot metal3-io-bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 4, 2025
@metal3-io-bot
Copy link
Contributor

Hi @tylerauerbeck. Thanks for your PR.

I'm waiting for a metal3-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@metal3-io-bot metal3-io-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 4, 2025
@tuminoid
Copy link
Member

tuminoid commented May 7, 2025

/ok-to-test

@metal3-io-bot metal3-io-bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 7, 2025
@tuminoid
Copy link
Member

tuminoid commented May 7, 2025

Thanks for the PR! AFAIK this does not break anything, any reason why it is marked as breaking change?

Also, fix the issues reported by CI and squash the commits.

@tylerauerbeck
Copy link
Contributor Author

@tuminoid Out of all the options, that seemed most appropriate since I was making changes to an underlying dependency. But happy to update it to anything you think is more appropriate.

@tylerauerbeck tylerauerbeck force-pushed the replace-errors-pkg branch 6 times, most recently from 68a0217 to 17aba85 Compare May 8, 2025 04:31
@tuminoid
Copy link
Member

tuminoid commented May 8, 2025

@tuminoid Out of all the options, that seemed most appropriate since I was making changes to an underlying dependency. But happy to update it to anything you think is more appropriate.

Thanks. Breaking is reserved to things that need changes on consumer or end-user side, like API version change, or major refactoring affecting lib consumers etc that they need to adapt when upgrading our module or release.

/retitle 🌱 Replace errors pkg with stdlib

@metal3-io-bot metal3-io-bot changed the title ⚠️ Replace errors pkg with stdlib 🌱 Replace errors pkg with stdlib May 8, 2025
tuminoid
tuminoid reviewed May 8, 2025
View reviewed changes
Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

/lgtm
/cc @lentzi90

@metal3-io-bot metal3-io-bot requested a review from lentzi90 May 8, 2025 08:51
@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label May 8, 2025
lentzi90
lentzi90 reviewed May 8, 2025
View reviewed changes
Copy link
Member

@lentzi90 lentzi90 left a comment

Choose a reason for hiding this comment

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

/approve

@metal3-io-bot metal3-io-bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. labels May 8, 2025
@tylerauerbeck tylerauerbeck mentioned this pull request May 12, 2025
Closed
@metal3-io-bot metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label May 12, 2025
@metal3-io-bot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@metal3-io-bot metal3-io-bot removed the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2025
@metal3-io-bot metal3-io-bot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 13, 2025
@metal3-io-bot metal3-io-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 13, 2025
@metal3-io-bot metal3-io-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 13, 2025
@tylerauerbeck tylerauerbeck force-pushed the replace-errors-pkg branch 2 times, most recently from 5df26a3 to b7abbd6 Compare May 13, 2025 02:20
@metal3-io-bot metal3-io-bot added the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2025
@metal3-io-bot metal3-io-bot removed the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2025
@tylerauerbeck
Copy link
Contributor Author

I don't think these tests failing are related to my changes as I see it failing in a few other tests. Does this happen to be an existing flake/broken tests? Just want to double check before digging in too deep.

fmuyassarov
fmuyassarov approved these changes May 13, 2025
View reviewed changes
Copy link
Member

@fmuyassarov fmuyassarov left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM
Thanks @tylerauerbeck .

@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fmuyassarov, lentzi90

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tuminoid
Copy link
Member

I don't think these tests failing are related to my changes as I see it failing in a few other tests. Does this happen to be an existing flake/broken tests? Just want to double check before digging in too deep.

Yes, @lentzi90 is taking look why BMO e2e is failing. Seems to be something external dependency changed.

@metal3-io-bot metal3-io-bot added the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label May 22, 2025
@lentzi90
Copy link
Member

The e2e tests should go green if you rebase this

@metal3-io-bot metal3-io-bot removed the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label May 27, 2025
@tylerauerbeck
Copy link
Contributor Author

@lentzi90 Should be good to go now.

@tuminoid
Copy link
Member

Squash commits please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tuminoid tuminoid tuminoid left review comments

@lentzi90 lentzi90 lentzi90 left review comments

@fmuyassarov fmuyassarov fmuyassarov approved these changes

@elfosardo elfosardo Awaiting requested review from elfosardo

@kashifest kashifest Awaiting requested review from kashifest

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Switch to errors of standard library and drop deprecated github.com/pkg/errors
5 participants
@tylerauerbeck @metal3-io-bot @tuminoid @lentzi90 @fmuyassarov