Skip to content
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

Social Data Endpoints and Constituent Exchanges #36

Merged
merged 2 commits into from
Feb 5, 2019

Conversation

kenchandev
Copy link
Contributor

@kenchandev kenchandev commented Jan 29, 2019

For the tests concerning the social data endpoints, if API_KEY is supplied via the CLI or environment variables, then these tests will run. If not, then these tests will not run. Hence, these additions shouldn't break the current Travis CI pipeline. Feel free to add a test API_KEY to the Travis CI pipeline's environment variables to enable these tests.

Here's a snapshot of the tests passing on my local machine:

If there's anything missing, then feel free to comment. I tried to adhere to the coding style in this repository based on the existing code.

Note: I wasn't able to update my forked repository with the latest merge due to git fetch upstream logging that I don't have the correct access rights. Therefore, ignore the first commit, which has already been merged previously.

@RyanZim
Copy link
Contributor

RyanZim commented Jan 29, 2019

Needs rebase to remove first commit.

@kenchandev
Copy link
Contributor Author

Oh yeah, I forget about rebasing 😅

Updated the PR so that it's only one commit now.

index.js Show resolved Hide resolved
@kenchandev
Copy link
Contributor Author

The PR has been fixed so that it's diffing with the latest version of the master branch. Sorry about the inconvenience. Git remains to be tricky even after all these years 😅

@RyanZim
Copy link
Contributor

RyanZim commented Jan 30, 2019

Can we get a note added to the docs about which methods require an API key?

@kenchandev
Copy link
Contributor Author

Can we get a note added to the docs about which methods require an API key?

Sure. Should this note be its own individual section in the documentation, or should it be included as a bullet point in each of the method sections?

@RyanZim
Copy link
Contributor

RyanZim commented Jan 30, 2019

Point on each method

@kenchandev
Copy link
Contributor Author

Added. Out of all of the endpoints included within this repo, only the social data endpoints require an API key (according to the Cryptocompare documentation).

@RyanZim RyanZim merged commit 8a420fd into ExodusMovement:master Feb 5, 2019
Sign up for free to 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.

2 participants