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.

Sign up for GitHub

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

Jump to bottom

Yahoo Finance v7 API (since 2017-05-16) - finish snapshot (2/3) #41

Merged
merged 5 commits into from
May 25, 2017
Merged

Yahoo Finance v7 API (since 2017-05-16) - finish snapshot (2/3) #41

merged 5 commits into from
May 25, 2017

Conversation

gadicc
Copy link
Collaborator

@gadicc gadicc commented May 25, 2017 •
edited
Loading

Continuation from #37 for snapshot().

  • snapshot()
    • Convert old API field options
    • Support symbols option (i.e. multiple symbols in one request - this needs multiple HTTP requests in yahoo's new API).
    • Ensure output is consistent with earlier API
    • convert dates

@gadicc gadicc changed the title [WIP] Continuation of new Yahoo API work [WIP] Yahoo Finance v7 API (since 2017-05-16) - final work May 25, 2017
@gadicc
split out snapshot and historical to their own files & tests
aece182 
historical: remove fields ref

historical.spec: remove snapshot assertion (to merge)
@gadicc
rename fields -> snapshotFields, for clarity
d1141dd
@pilwon pilwon mentioned this pull request May 25, 2017
Closed
@gadicc gadicc changed the title [WIP] Yahoo Finance v7 API (since 2017-05-16) - final work Yahoo Finance v7 API (since 2017-05-16) - finish snapshot() May 25, 2017
@gadicc
Copy link
Collaborator Author

gadicc commented May 25, 2017 •
edited
Loading

@pilwon, ok, changed my mind :) This PR is just for snapshot(). Sorry, I just saw all your references to this issue but your earlier comments about shipping as early as possible have been stuck in my mind and I think this is ready to ship and might help some people.

Take a look through the changes and please try any of your own tests if you have any. You can ignore my merge messages in the commits and just squash everything. Everything else really should be in one final PR, which is less time sensitive. I'll update all the other threads with links when I open it.

@gadicc
Copy link
Collaborator Author

gadicc commented May 25, 2017

oh actually just wait one sec, sorry.

@gadicc
package.json: add sinon, sinon-chai
582f46b
@gadicc
snapshot: update map, transform code, warnings
86b6b95 
snapshot: arrays & dates (to merge with above)

snapshot: remove commented out code (to merge with above)

snapshot: fix module requests, add test (merge with above)

snapshot: fix modules requests, integration test (merge above)

snapshot: multi symbol support + tests (merge above)

snapshot: integration test, remove .only (merge above)
@gadicc
README: update New Yahoo API section, particularly snapshot()
59f2e99
@gadicc
Copy link
Collaborator Author

gadicc commented May 25, 2017

Ok I just rebased on my end down to 5 sensible commits, which I think you can merge now without squashing. It's a lot clearer to have i.e. file splits and unrelated changes in their own commits.

@pilwon pilwon merged commit 7bdc0dc into pilwon:master May 25, 2017
@pilwon
Copy link
Owner

pilwon commented May 25, 2017

@gadicc Awesome! 👍 👍 👍

@gadicc gadicc mentioned this pull request May 26, 2017
Merged
10 tasks
@gadicc gadicc changed the title Yahoo Finance v7 API (since 2017-05-16) - finish snapshot() Yahoo Finance v7 API (since 2017-05-16) - 2/3 - finish snapshot() May 26, 2017
@gadicc gadicc changed the title Yahoo Finance v7 API (since 2017-05-16) - 2/3 - finish snapshot() Yahoo Finance v7 API (since 2017-05-16) - finish snapshot (2/3) May 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gadicc @pilwon