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

Improve MsgTransfer #568

Merged
merged 3 commits into from
Mar 29, 2023
Merged

Improve MsgTransfer #568

merged 3 commits into from
Mar 29, 2023

Conversation

plafer
Copy link
Contributor

@plafer plafer commented Mar 27, 2023

Closes: #567


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests.
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

@codecov
Copy link

codecov bot commented Mar 27, 2023

Codecov Report

Patch coverage: 81.25% and project coverage change: -0.04 ⚠️

Comparison is base (8cc943d) 73.02% compared to head (5b02b15) 72.99%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #568      +/-   ##
==========================================
- Coverage   73.02%   72.99%   -0.04%     
==========================================
  Files         126      126              
  Lines       15679    15672       -7     
==========================================
- Hits        11450    11439      -11     
- Misses       4229     4233       +4     
Impacted Files Coverage Δ
...tes/ibc/src/applications/transfer/msgs/transfer.rs 45.79% <53.57%> (-0.75%) ⬇️
...c/src/applications/transfer/relay/send_transfer.rs 95.41% <96.07%> (-0.42%) ⬇️
crates/ibc/src/core/handler.rs 74.64% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@Farhad-Shabani Farhad-Shabani left a comment

Choose a reason for hiding this comment

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

All set.
There is just an inconsistency with the naming of some fields and methods I remember you also mentioned this before. Some places we use "token" and other times we use "coin". Do we need to deal with that in a later PR?

@plafer
Copy link
Contributor Author

plafer commented Mar 29, 2023

yes I can always do that in the next PR 👍

@plafer plafer merged commit 1cf4423 into main Mar 29, 2023
@plafer plafer deleted the plafer/567-msgtransfer branch March 29, 2023 20:52
Farhad-Shabani pushed a commit that referenced this pull request Sep 9, 2024
* first cleanup

* improve MsgTransfer

* changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve MsgTransfer struct
2 participants