Skip to content
This repository has been archived by the owner on Sep 28, 2022. It is now read-only.

CDP-484 add method to fetch ilk by cdp ID #190

Merged
merged 3 commits into from
Nov 26, 2019
Merged

Conversation

b-pmcg
Copy link
Contributor

@b-pmcg b-pmcg commented Nov 22, 2019

No description provided.

@b-pmcg b-pmcg requested a review from levity November 22, 2019 16:11
@b-pmcg
Copy link
Contributor Author

b-pmcg commented Nov 22, 2019

see OasisDEX/mcd-cdp-portal#156

@codecov-io
Copy link

codecov-io commented Nov 22, 2019

Codecov Report

Merging #190 into dev will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##             dev    #190      +/-   ##
========================================
+ Coverage   92.5%   92.5%   +<.01%     
========================================
  Files         87      87              
  Lines       2441    2442       +1     
========================================
+ Hits        2258    2259       +1     
  Misses       183     183
Impacted Files Coverage Δ
packages/dai-plugin-mcd/src/CdpManager.js 96.64% <100%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae1b2a0...a1c6f13. Read the comment docs.

Copy link
Contributor

@levity levity left a comment

Choose a reason for hiding this comment

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

I'm still not sure this is necessary for OasisDEX/mcd-cdp-portal#156, but it seems like it might come in handy at some point anyway. The only thing I would suggest is renaming it to getIlkForCdp. Might seem like a trivial change, but this wording suggests to me that there is a single ilk for a cdp, which is the case, whereas getIlkByCdpId makes it sound like cdp id is a property of an ilk.

@b-pmcg b-pmcg merged commit af59198 into dev Nov 26, 2019
@b-pmcg b-pmcg deleted the CDP-484-getIlkByCdpId branch December 11, 2019 17:31
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.

3 participants