tendermint / spn

A blockchain to launch blockchains.
Other
112 stars 41 forks source link

refactor: remove `sdkerrortypes` #967

Closed aljo242 closed 1 year ago

aljo242 commented 1 year ago

Remove all imports of sdkerrortypes from the repo. All usages are replaced by appropriate module-scoped error types. Some errors are repeated between modules, but scoping them to the modules, gives a bit more context. It also means that any clients or other code importing our code will just have to use our error types and no other 3rd party errors.

codecov[bot] commented 1 year ago

Codecov Report

Merging #967 (513c88f) into develop (2345454) will increase coverage by 0.00%. The diff coverage is 69.86%.

Additional details and impacted files [![Impacted file tree graph](https://codecov.io/gh/tendermint/spn/pull/967/graphs/tree.svg?width=650&height=150&src=pr&token=GMSSU7SM3A&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint)](https://codecov.io/gh/tendermint/spn/pull/967?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint) ```diff @@ Coverage Diff @@ ## develop #967 +/- ## ======================================== Coverage 10.54% 10.55% ======================================== Files 275 275 Lines 66486 66482 -4 ======================================== + Hits 7011 7016 +5 + Misses 59304 59297 -7 + Partials 171 169 -2 ``` | [Impacted Files](https://codecov.io/gh/tendermint/spn/pull/967?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint) | Coverage Δ | | |---|---|---| | [x/monitoringc/module\_ibc.go](https://codecov.io/gh/tendermint/spn/pull/967/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint#diff-eC9tb25pdG9yaW5nYy9tb2R1bGVfaWJjLmdv) | `0.00% <0.00%> (ø)` | | | [x/monitoringp/keeper/monitoring\_packet.go](https://codecov.io/gh/tendermint/spn/pull/967/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint#diff-eC9tb25pdG9yaW5ncC9rZWVwZXIvbW9uaXRvcmluZ19wYWNrZXQuZ28=) | `8.19% <0.00%> (ø)` | | | [x/monitoringp/module\_ibc.go](https://codecov.io/gh/tendermint/spn/pull/967/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint#diff-eC9tb25pdG9yaW5ncC9tb2R1bGVfaWJjLmdv) | `0.00% <0.00%> (ø)` | | | [x/campaign/keeper/msg\_create\_campaign.go](https://codecov.io/gh/tendermint/spn/pull/967/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint#diff-eC9jYW1wYWlnbi9rZWVwZXIvbXNnX2NyZWF0ZV9jYW1wYWlnbi5nbw==) | `86.95% <100.00%> (ø)` | | | [x/campaign/types/msg\_burn\_vouchers.go](https://codecov.io/gh/tendermint/spn/pull/967/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint#diff-eC9jYW1wYWlnbi90eXBlcy9tc2dfYnVybl92b3VjaGVycy5nbw==) | `45.45% <100.00%> (ø)` | | | [x/campaign/types/msg\_create\_campaign.go](https://codecov.io/gh/tendermint/spn/pull/967/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint#diff-eC9jYW1wYWlnbi90eXBlcy9tc2dfY3JlYXRlX2NhbXBhaWduLmdv) | `45.71% <100.00%> (ø)` | | | [x/campaign/types/msg\_edit\_campaign.go](https://codecov.io/gh/tendermint/spn/pull/967/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint#diff-eC9jYW1wYWlnbi90eXBlcy9tc2dfZWRpdF9jYW1wYWlnbi5nbw==) | `47.22% <100.00%> (ø)` | | | [x/campaign/types/msg\_initialize\_mainnet.go](https://codecov.io/gh/tendermint/spn/pull/967/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint#diff-eC9jYW1wYWlnbi90eXBlcy9tc2dfaW5pdGlhbGl6ZV9tYWlubmV0Lmdv) | `42.85% <100.00%> (ø)` | | | [x/campaign/types/msg\_mint\_vouchers.go](https://codecov.io/gh/tendermint/spn/pull/967/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint#diff-eC9jYW1wYWlnbi90eXBlcy9tc2dfbWludF92b3VjaGVycy5nbw==) | `40.00% <100.00%> (ø)` | | | [x/campaign/types/msg\_redeem\_vouchers.go](https://codecov.io/gh/tendermint/spn/pull/967/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint#diff-eC9jYW1wYWlnbi90eXBlcy9tc2dfcmVkZWVtX3ZvdWNoZXJzLmdv) | `50.00% <100.00%> (ø)` | | | ... and [26 more](https://codecov.io/gh/tendermint/spn/pull/967/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tendermint) | |
aljo242 commented 1 year ago

I thought as well, but then we're doing the same thing - sometimes importing errors from another package (our own now) and sometimes using errors we define in our types package. Using the ignite errors also moves the codespace to some other spot (here "ignite") which isn't really giving any useful information. With this PR, every module only throws errors in its own codespace, or critical errors.

This is also more in line with the SDK design going forward, since they are deprecating those errors.

lumtis commented 1 year ago

I thought as well, but then we're doing the same thing - sometimes importing errors from another package (our own now) and sometimes using errors we define in our types package. Using the ignite errors also moves the codespace to some other spot (here "ignite") which isn't really giving any useful information. With this PR, every module only throws errors in its own codespace, or critical errors.

This is also more in line with the SDK design going forward, since they are deprecating those errors.

Ok, this makes sense What about completely removing https://github.com/ignite/modules/blob/main/pkg/errors/errors.go then? and do the same in modules?