Skip to content
This repository has been archived by the owner on May 30, 2023. It is now read-only.

feat: omnipool imbalance adjustment #87

Merged
merged 5 commits into from
Jan 23, 2023
Merged

Conversation

enthusiastmartin
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Jan 23, 2023

Codecov Report

Base: 90.53% // Head: 90.81% // Increases project coverage by +0.28% 🎉

Coverage data is based on head (5d9d419) compared to base (b454b72).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #87      +/-   ##
==========================================
+ Coverage   90.53%   90.81%   +0.28%     
==========================================
  Files          10       10              
  Lines         697      697              
==========================================
+ Hits          631      633       +2     
+ Misses         66       64       -2     
Impacted Files Coverage Δ
src/omnipool/math.rs 91.94% <100.00%> (ø)
src/omnipool/types.rs 100.00% <0.00%> (+4.44%) ⬆️

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 at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@poliwop
Copy link
Contributor

poliwop commented Jan 23, 2023

Not a request to change anything, just a note after thinking about how we missed this:
It's harder to test these objects we're returning where we are specifying the deltas, it would be much easier to test objects where we just specified the new state for the variables involved (i.e. we specified the new value for L instead of delta L). This is especially true with Increase() and Decrease() custom enums.

Probably this is just something to keep in mind for the future...

@poliwop
Copy link
Contributor

poliwop commented Jan 23, 2023

I should mention I did review this change and it looks good to me, won't do a formal "review" since I'm not so familiar with Rust.

@enthusiastmartin
Copy link
Contributor Author

Not a request to change anything, just a note after thinking about how we missed this: It's harder to test these objects we're returning where we are specifying the deltas, it would be much easier to test objects where we just specified the new state for the variables involved (i.e. we specified the new value for L instead of delta L). This is especially true with Increase() and Decrease() custom enums.

Probably this is just something to keep in mind for the future...

yes. i kind of agree that imbalance update is little bit confusing.

i will suggest refactoring this part .

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

Successfully merging this pull request may close these issues.

None yet

3 participants