Create function coinList() to get list of coins from CryptoCompare API by owencraston · Pull Request #10 · ExodusOSS/cryptocompare

owencraston · 2017-11-17T06:39:17Z

I needed to get a list of coins from CryptoCompare so I thought I'd abstract it into this library.

the coin list data is now available at https://min-api.cryptocompare.com/data/all/coinlist so we can get coinList again.
more information can be found here:
https://www.cryptocompare.com/api/#-api-data-coinlist-
write tests for this function
add documentation for this function
updated CHANGELOG

RyanZim

Tests & linter are failing, please fix this. I left two comments below pointing out some of the problems.

RyanZim · 2017-11-17T16:35:54Z

    })
 }

+function coinList() {


This function needs to be exported at the bottom of the file.

RyanZim · 2017-11-17T16:36:13Z

 const helpers = require('./helpers')

+test('coinList()', t => {
+  t.plan(2)


This will make tests fail.

owencraston · 2017-11-17T18:44:00Z

@RyanZim tests seem to still be failing? Is this a problem with the node version?

RyanZim · 2017-11-18T15:25:27Z

@owencraston The linter errors still aren't fixed, please run npm test locally and get everything fixed up before pushing here.

owencraston · 2017-11-22T19:00:25Z

@RyanZim Does this look good to you?

RyanZim

Looks good overall; though I'm not sure if we want package-lock.json committed to the repo; @jprichardson?

RyanZim

Sorry, reviewed too quickly, there are broken tests here. Our travis config was messed up, so travis didn't run all the tests right. I fixed that in master.

Please rebase this onto master, and fix the tests as outlined below. Also talked to JP about this, we don't want the package-lock.json committed here, so remove that; also remove your changes to CHANGELOG while you're at it.

RyanZim · 2017-11-22T19:43:50Z

+    t.strictEqual(typeof coins.Data.ETH.Name, 'string', 'coins.Data.ETH.Name` is a string')
+    t.strictEqual(typeof coins.Data.ETH.Name, 'ETH', 'coins.Data.ETH.Name is \'ETH\'')
+    t.strictEqual(typeof coins.Data.ETH.FullName, 'Ethereum (ETH)', 'coins.Data.ETH.FullName is \'Ethereum (ETH)\'')
+    t.strictEqual(typeof coins.Data.ETH.Sponsored, false, 'coins.Data.ETH.Sponsored is \'false\'')


You don't want typeof here, except for the top assertion.

owencraston · 2017-11-22T19:48:27Z

Based on this doc I think I should remove it: https://docs.npmjs.com/files/package-lock.json
One key detail about package-lock.json is that it cannot be published, and it will be ignored if found in any place other than the toplevel package. It shares a format with npm-shrinkwrap.json, which is essentially the same file, but allows publication. This is not recommended unless deploying a CLI tool or otherwise using the publication process for producing production packages.
We should probably add this to the .gitignore file.

owencraston · 2017-11-27T07:06:00Z

Why are these tests still failing? @RyanZim

RyanZim · 2017-11-27T15:53:31Z

Not sure, will take a look later.

RyanZim

Your URL is wrong; that's why the tests are failing.

RyanZim · 2017-11-27T18:14:32Z

 }

+function coinList () {
+  const url = `${baseUrl}coinlist/`


Should be:

const url = `${baseUrl}all/coinlist`

Right. I made this change but the tests are still failing.

RyanZim · 2017-11-27T20:07:44Z

@owencraston You need to remove the trailing slash from the URL. CryptoCompare doesn't like it.

- the coin list data is now available at https://min-api.cryptocompare.com/data/all/coinlist - more information can be found here: https://www.cryptocompare.com/api/#-api-data-coinlist- - write tests for this function - add documentation

owencraston · 2017-11-27T20:32:52Z

Thanks for all the help @RyanZim. Sorry this took so long. I think it should be good to go! I rebased all the commits into one atomic commit just for simplicity.

RyanZim

utACK LGTM, @jprichardson?

jprichardson · 2017-11-27T22:02:42Z

@RyanZim please release update to npm.

jprichardson · 2017-11-27T22:02:52Z

Thanks @owencraston!

owencraston · 2017-11-27T22:03:35Z

🚀

RyanZim · 2017-11-28T00:43:54Z

Released as 0.3.0 🎉

owencraston force-pushed the cryptocompare-add-coinlist branch from e7d9603 to 9200b6f Compare November 17, 2017 06:40

PhearZero mentioned this pull request Nov 17, 2017

Adding coinList function to complete API (more tests required) #9

Closed

RyanZim suggested changes Nov 17, 2017

View reviewed changes

owencraston force-pushed the cryptocompare-add-coinlist branch from ba1b036 to fd51081 Compare November 22, 2017 18:55

RyanZim approved these changes Nov 22, 2017

View reviewed changes

RyanZim suggested changes Nov 22, 2017

View reviewed changes

owencraston force-pushed the cryptocompare-add-coinlist branch from fd51081 to 8150d8a Compare November 27, 2017 06:45

RyanZim self-assigned this Nov 27, 2017

RyanZim reviewed Nov 27, 2017

View reviewed changes

owencraston force-pushed the cryptocompare-add-coinlist branch from 8150d8a to 25b53bf Compare November 27, 2017 19:21

owencraston force-pushed the cryptocompare-add-coinlist branch from 25b53bf to 8c7becb Compare November 27, 2017 20:31

RyanZim approved these changes Nov 27, 2017

View reviewed changes

jprichardson merged commit edd64d1 into ExodusOSS:master Nov 27, 2017

owencraston deleted the cryptocompare-add-coinlist branch November 27, 2017 22:03

Conversation

owencraston commented Nov 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RyanZim left a comment

Choose a reason for hiding this comment

Uh oh!

RyanZim Nov 17, 2017

Choose a reason for hiding this comment

Uh oh!

RyanZim Nov 17, 2017

Choose a reason for hiding this comment

Uh oh!

owencraston commented Nov 17, 2017

Uh oh!

RyanZim commented Nov 18, 2017

Uh oh!

owencraston commented Nov 22, 2017

Uh oh!

RyanZim left a comment

Choose a reason for hiding this comment

Uh oh!

RyanZim left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RyanZim Nov 22, 2017

Choose a reason for hiding this comment

Uh oh!

owencraston commented Nov 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

owencraston commented Nov 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RyanZim commented Nov 27, 2017

Uh oh!

RyanZim left a comment

Choose a reason for hiding this comment

Uh oh!

RyanZim Nov 27, 2017

Choose a reason for hiding this comment

Uh oh!

owencraston Nov 27, 2017

Choose a reason for hiding this comment

Uh oh!

RyanZim commented Nov 27, 2017

Uh oh!

owencraston commented Nov 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RyanZim left a comment

Choose a reason for hiding this comment

Uh oh!

jprichardson commented Nov 27, 2017

Uh oh!

jprichardson commented Nov 27, 2017

Uh oh!

owencraston commented Nov 27, 2017

Uh oh!

RyanZim commented Nov 28, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

owencraston commented Nov 17, 2017 •

edited

Loading

RyanZim left a comment •

edited

Loading

owencraston commented Nov 22, 2017 •

edited

Loading

owencraston commented Nov 27, 2017 •

edited

Loading

owencraston commented Nov 27, 2017 •

edited

Loading