[chore] Replaces multierr usage with go native errors package #939
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
20 tasks
Olshansk
requested changes
Jul 26, 2023
| @@ -197,6 +196,7 @@ require ( | |||
| go.uber.org/atomic v1.10.0 // indirect | |||
| go.uber.org/dig v1.15.0 // indirect | |||
| go.uber.org/fx v1.18.2 // indirect | |||
| go.uber.org/multierr v1.9.0 // indirect | |||
There was a problem hiding this comment.
Can we remove this dependency altogether? (go.sum) should be updated too
There was a problem hiding this comment.
I assume another import from uber uses the multierr as a dep so we cannot get rid of it altogether until they update
There was a problem hiding this comment.
It's being imported by some other dependency of ours, but there are no more usages of multierr in our specific code that I could find.
There was a problem hiding this comment.
Can we remove this dependency altogether? (go.sum) should be updated too
I actually don't think go.sum needs to change, the dependency was only made indirect, and the version stayed the same, so I suspect the dependency graph stayed the same 🤔
h5law
reviewed
Jul 26, 2023
h5law
reviewed
Jul 26, 2023
h5law
approved these changes
Jul 26, 2023
dylanlott
force-pushed
the
persistence/savepoints-initial
branch
from
9a3c346
to
29ad7df
Compare
* fixes treestore module tests to use the bus for fetching the height provider instead of accessing it from the package
Small commit to reduce visibility of a type for in progress PR
dylanlott
force-pushed
the
persistence/savepoints-initial
branch
from
29ad7df
to
0f87f0b
Compare
dylanlott
force-pushed
the
chore/remove-multierr
branch
from
35226dd
to
f34351f
Compare
Olshansk
approved these changes
Jul 28, 2023
dylanlott
force-pushed
the
chore/remove-multierr
branch
from
c1542a0
to
7dffd28
Compare
dylanlott
removed
the
do not merge
red-0ne
pushed a commit
that referenced
this pull request
Aug 2, 2023
Co-authored-by: Daniel Olshansky <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The recent upgrade to Go 1.20 enabled the errors package
Joinfunction to replace usage of multierrCombineandAppendfunctions. Please note it cannot be removed as a dependency from ourgo.modbecause it is an indirect dependency.Issue
Follow up changes from review of #861 and merging #910
Type of change
Please mark the relevant option(s):
List of changes
multierr.Combineandmultierr.Appendusage witherrors.JoinTesting
make develop_test; if any code changes were mademake test_e2eon k8s LocalNet; if any code changes were madee2e-devnet-testpasses tests on DevNet; if any code was changedRequired Checklist
godocformat comments on touched members (see: tip.golang.org/doc/comment)If Applicable Checklist
shared/docs/*if I updatedshared/*README(s)