-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat: standalone errors go.mod #10779
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK, can we add a changelog entry
added. does it make sense that this goes under also, I added a CHANGELOG for the |
@@ -0,0 +1,11 @@ | |||
module errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happened to the core
/base
idea discussed in #9011?
in my mind errors
has too small scope to warrant its own module. Same applies to math
or codec
. I personally prefer to have one core
module to regroup these small building blocks, instead of having to maintain 10+ small go modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to sort of agree here. What are your thoughts @aaronc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to write up my thoughts in a discussion thread and link back here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Posted my thoughts here: #10582 (comment). Would be great to hear your comments on this
@AmauryM @alexanderbez @robert-zaremba what are your thoughts on allowing this to be merged for now but continuing the discussion in the new year. |
SGTM 👍. If we tag, maybe we can tag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, pending tests
How about beta because the SDK will depend on it? |
beta is fine too |
Codecov Report
@@ Coverage Diff @@
## master #10779 +/- ##
==========================================
- Coverage 66.00% 64.58% -1.43%
==========================================
Files 632 611 -21
Lines 63334 58741 -4593
==========================================
- Hits 41805 37936 -3869
+ Misses 19199 18673 -526
+ Partials 2330 2132 -198 |
Description
This PR:
types/errors
code to a newerrors
go module, except:RootCodespace
errors intypes/errors
stay theretypes/errors
referencingerrors
so this is not a breaking changeThis will allow standalone go modules to use the same error types as the SDK. In particular, I want the
orm
to referenceerrors
and then the SDK will be able to importorm
and it can stay standalone. The same could apply to thedb
module.After this PR the plan is to:
github.com/cosmos/cosmos-sdk/errors
asv1.0
🎉replace
directive forerrors
in the main SDKgo.mod
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change