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.

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

refactor: replace deprecated errors module in src #12729

Merged
merged 2 commits into from May 30, 2022
Merged

refactor: replace deprecated errors module in src #12729

merged 2 commits into from May 30, 2022

Conversation

MoritzThomasHuebner
Copy link
Contributor

@MoritzThomasHuebner MoritzThomasHuebner commented May 16, 2022 •
edited

Summary

See #12632 for description

Remove dependency on "github.com/pkg/errors"

  • Replace use of errors.Wrap[f] with fmt.Errorf
  • Replace use of pkg errors.New with standard lib errors.New
  • Replace a use of pkg errors.Cause with standard lib errors.Unwrap
  • Update go.mod files

Test Plan

Run unit tests

Additional Information

Worked in pairing with @wolfseb

  • This change is backwards-breaking

@pull-request-size pull-request-size bot added the size/L Denotes a Pull Request that changes 100-499 lines. label May 16, 2022
@github-actions
Copy link
Contributor

Thanks for opening a PR! 💯

A couple initial guidelines

Howto

  • Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
  • Checks. All required CI checks must pass before merge.
  • Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

If this is your first Magma PR, also consider reading

@MoritzThomasHuebner MoritzThomasHuebner marked this pull request as draft May 16, 2022 06:48
@maxhbr maxhbr mentioned this pull request May 16, 2022
Closed
6 tasks
@MoritzThomasHuebner MoritzThomasHuebner changed the title refactor(src): Replaces errors.Wrap[f] with fmt.Errorf. refactor: Replaces errors.Wrap[f] with fmt.Errorf in src May 16, 2022
@MoritzThomasHuebner MoritzThomasHuebner marked this pull request as ready for review May 16, 2022 08:44
@MoritzThomasHuebner MoritzThomasHuebner changed the title refactor: Replaces errors.Wrap[f] with fmt.Errorf in src refactor: replace deprecated errors module in src May 17, 2022
@MoritzThomasHuebner MoritzThomasHuebner added ready2merge This PR is ready to be merged (is approved and passes all required checks) and removed ready2merge This PR is ready to be merged (is approved and passes all required checks) labels May 18, 2022
@MoritzThomasHuebner MoritzThomasHuebner requested review from a team and uri200 May 24, 2022 02:02
@github-actions
Copy link
Contributor

github-actions bot commented May 24, 2022 •
edited

dp-workflow

  2 files    2 suites   3m 44s ⏱️
15 tests 15 ✔️ 0 💤 0 ❌

Results for commit 63a7be4.

♻️ This comment has been updated with latest results.

@MoritzThomasHuebner MoritzThomasHuebner added ready2merge This PR is ready to be merged (is approved and passes all required checks) and removed ready2merge This PR is ready to be merged (is approved and passes all required checks) labels May 24, 2022
maxhbr
maxhbr approved these changes May 25, 2022
View reviewed changes
Copy link
Member

@maxhbr maxhbr left a comment

Choose a reason for hiding this comment

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

LGTM, except for the one non-documented resolution.

src/go/agwd/config/config_test.go
@@ -258,7 +258,7 @@ func TestNewConfigManager_DefaultNotFound(t *testing.T) {

cm := NewConfigManager()
err := LoadConfigFile(cm, filepath.Join("testdata", "doesnotexist.json"))
assert.True(t, os.IsNotExist(errors.Cause(err)))
assert.True(t, os.IsNotExist(errors.Unwrap(err)))
Copy link
Member

Choose a reason for hiding this comment

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

This is not discussed in #12632, can you add it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a bullet point to the list in my comment there.

@MoritzThomasHuebner MoritzThomasHuebner added the ready2merge This PR is ready to be merged (is approved and passes all required checks) label May 25, 2022
@MoritzThomasHuebner
refactor(src): Replaces errors.Wrap[f] with fmt.Errorf.
456ad21 
Signed-off-by: Moritz Huebner <moritz.huebner@tngtech.com>
@MoritzThomasHuebner
refactor(src): Replaces more pkg/errors usages with std lib.
63a7be4 
Also update go.mod file

Signed-off-by: Moritz Huebner <moritz.huebner@tngtech.com>
@sebathomas sebathomas merged commit be8f77d into magma:master May 30, 2022
mpfirrmann pushed a commit to mpfirrmann/magma that referenced this pull request May 31, 2022
@MoritzThomasHuebner @mpfirrmann
refactor: replace deprecated errors module in src (magma#12729)
109050c 
* refactor(src): Replaces errors.Wrap[f] with fmt.Errorf.

Signed-off-by: Moritz Huebner <moritz.huebner@tngtech.com>

* refactor(src): Replaces more pkg/errors usages with std lib.

Also update go.mod file

Signed-off-by: Moritz Huebner <moritz.huebner@tngtech.com>
emakeev pushed a commit to emakeev/magma that referenced this pull request Aug 5, 2022
@MoritzThomasHuebner @emakeev
refactor: replace deprecated errors module in src (magma#12729)
775ff99 
* refactor(src): Replaces errors.Wrap[f] with fmt.Errorf.

Signed-off-by: Moritz Huebner <moritz.huebner@tngtech.com>

* refactor(src): Replaces more pkg/errors usages with std lib.

Also update go.mod file

Signed-off-by: Moritz Huebner <moritz.huebner@tngtech.com>
@MoritzThomasHuebner MoritzThomasHuebner deleted the Remove_pkg/errors_usage_from_src branch February 23, 2023 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxhbr maxhbr maxhbr approved these changes

@Neudrino Neudrino Neudrino approved these changes

@alexzurbonsen alexzurbonsen alexzurbonsen approved these changes

@markjen markjen Awaiting requested review from markjen

@uri200 uri200 Awaiting requested review from uri200

Assignees
No one assigned
Labels
ready2merge This PR is ready to be merged (is approved and passes all required checks) size/L Denotes a Pull Request that changes 100-499 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@MoritzThomasHuebner @maxhbr @Neudrino @alexzurbonsen @sebathomas