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.

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

Unify text format for invalid/unexpected value errors #334

Open
3 tasks
crodriguezvega opened this issue Aug 13, 2021 · 3 comments
Open
3 tasks

Unify text format for invalid/unexpected value errors #334

crodriguezvega opened this issue Aug 13, 2021 · 3 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@crodriguezvega
Copy link
Contributor

Summary

The error message for an invalid value is formatted in slightly different ways. Take the following three as an example:

"expected %s channel, got %s " (https://github.com/cosmos/ibc-go/blob/main/modules/apps/transfer/module.go#L207)
"invalid port: %s, expected %s" (https://github.com/cosmos/ibc-go/blob/main/modules/apps/transfer/module.go#L213)
"got %s, expected %s" (https://github.com/cosmos/ibc-go/blob/main/modules/apps/transfer/module.go#L217)

The goal is to format this kind of errors in a consistent way.

The proposal is to do the following:

sdkerrors.Wrapf(
  <most specific error type possible>,
  "<optional text description ended by colon and space>expected %s, got %s",
  <value 1>,
  <value 2>
)

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@crodriguezvega crodriguezvega added the good first issue Good for newcomers label Aug 13, 2021
@crodriguezvega crodriguezvega self-assigned this Aug 13, 2021
faddat pushed a commit to notional-labs/ibc-go that referenced this issue Mar 1, 2022
CosmosCar pushed a commit to caelus-labs/ibc-go that referenced this issue Nov 6, 2023
Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.7.0 to 1.7.1.
- [Release notes](https://github.com/stretchr/testify/releases)
- [Commits](stretchr/testify@v1.7.0...v1.7.1)

---
updated-dependencies:
- dependency-name: github.com/stretchr/testify
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@iricehasan
Copy link

Hello @crodriguezvega , I would like to do this if this is still needed. Can you assign me this issue?

@crodriguezvega
Copy link
Contributor Author

Sorry for the late reply, @iricehasan. I hope you're still interested in the issue, I will assign it to you now.

@iricehasan
Copy link

Hi @crodriguezvega, I have just seen your reply. I am still interested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants