Skip to content

CurrencyRatesController: add baseAsset support - #30

Merged
bitpshr merged 5 commits into
masterfrom
currency-rates-cleanup
Nov 27, 2018
Merged

CurrencyRatesController: add baseAsset support#30
bitpshr merged 5 commits into
masterfrom
currency-rates-cleanup

Conversation

@bitpshr

Copy link
Copy Markdown
Contributor

This pull request cleans up CurrencyRatesController and adds support for a new configuration parameter: baseAsset.

@codecov-io

codecov-io commented Nov 27, 2018

Copy link
Copy Markdown

Codecov Report

Merging #30 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@ Coverage Diff @@## master #30 +/- ##
=====================================
Coverage 100% 100% =====================================
Files 18 18 Lines 889 891 +2 Branches 97 97 =====================================
+ Hits 889 891 +2
Impacted FilesCoverage Δ
src/CurrencyRateController.ts100% <100%> (ø)⬆️

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 e2098be...c1046d9. Read the comment docs.

@bitpshr
bitpshrforce-pushed the currency-rates-cleanup branch 2 times, most recently from c71b81f to f93194eCompareNovember 27, 2018 17:13
@bitpshr
bitpshrforce-pushed the currency-rates-cleanup branch from f93194e to f3024adCompareNovember 27, 2018 17:44
Comment threadsrc/CurrencyRateController.test.ts Outdated
const baseAsset = 'FOO';
const controller = new CurrencyRateController({ baseAsset });
const mock = stub(window, 'fetch');
(window.fetch as SinonStub).returns(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we use mock here instead of casting window.fetch? mock should have .returns on it, I think.

Also we might be able to use .resolves(x) instead of .returns(Promise.resolve(x)) here.

@brunobar79brunobar79 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Just one question about deps.

Comment threadpackage.json
"human-standard-token-abi": "^2.0.0",
"isomorphic-fetch": "^2.2.1",
"percentile": "^1.2.1",
"uuid": "^3.3.2",

@brunobar79brunobar79Nov 27, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I couldn't find where you are using this dep. in this PR

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

We were missing it, it's used in TransactionController.

@bitpshr
bitpshrforce-pushed the currency-rates-cleanup branch from 90b9038 to c9e55e0CompareNovember 27, 2018 20:35
@bitpshr
bitpshr merged commit 7472822 into masterNov 27, 2018
@bitpshr
bitpshr deleted the currency-rates-cleanup branch November 27, 2018 20:48
mcmire pushed a commit to mcmire/core that referenced this pull request Jul 17, 2023
* 3.0.3
* Update CHANGELOG.md
* Update CHANGELOG.md
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
kanthesha pushed a commit that referenced this pull request Sep 19, 2023
Mrtenz pushed a commit that referenced this pull request Oct 16, 2025
Mrtenz pushed a commit to Mrtenz/core that referenced this pull request Jun 17, 2026
Sign up for freeto 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.

4 participants

@bitpshr@codecov-io@brunobar79@whymarrh