-
Notifications
You must be signed in to change notification settings - Fork 302
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
Implement NamedMultiReservableCurrency and NamedBasicReservableCurrency in orml_currencies #743
Implement NamedMultiReservableCurrency and NamedBasicReservableCurrency in orml_currencies #743
Conversation
Ok. Simply add the trait bound makes no sense (that's what I read from the issue) but actually implement |
@xlc I don't get why the cargo fmt isn't correct. I formatted it multiple times. |
Codecov Report
@@ Coverage Diff @@
## master #743 +/- ##
==========================================
- Coverage 77.68% 77.17% -0.52%
==========================================
Files 92 92
Lines 9069 9129 +60
==========================================
Hits 7045 7045
- Misses 2024 2084 +60
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
I let you finish the work. I think the most is done. |
I don't really know if this might be a problem in the future. I mean we got a PS: I hate this cargo fmt problem here in this repo... |
Update your cargo to a recent version should resolve the fmt issue. This is one used by CI
|
Multiple |
Thanks @xlc ! If there is anything to correct in my pull request, I am open to correct it after a review. |
LGTM except missing testcase, can you do this here or maybe in another PR? @Chralt98 |
@zqhxuyuan This depends on, what would be faster for a merge (Zeitgeist requires this functionality as fast as possible). I would propose, that it should get another pull request. But please verify the code before merge again :-D. I don't like to impact many chains to loose money. I would try to add test cases later in the day in another pull request. But I would be happy, if you could spend time on this too. |
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
It's better to add |
@zqhxuyuan You mean |
ok, my bad. the |
Hey, this is related to #742.
It's the first try and it's not ready yet.