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

added a historical quote function to Quote #11

Merged
merged 6 commits into from
Jan 31, 2019

Conversation

jacks821
Copy link
Contributor

This only gives the closing price of the requested day.

@coveralls
Copy link

coveralls commented Dec 27, 2018

Pull Request Test Coverage Report for Build 275

  • 13 of 13 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 84.855%

Totals Coverage Status
Change from base Build 210: 0.2%
Covered Lines: 846
Relevant Lines: 997

💛 - Coveralls

@ackleymi
Copy link
Contributor

@jacks821 looks pretty good. Thanks for taking this on, I've been busy with some other projects. Can you look into the failing checks? Also, for full integration tests, I suppose we'll need a modification to the test server project https://github.com/piquette/finance-mock (annoying I know). Let me know if you have any thoughts. Maybe a unit test will suffice?

@jacks821
Copy link
Contributor Author

I appreciate the feedback! I'm gonna do that and re up the commit!

@ackleymi
Copy link
Contributor

@jacks821 maybe add some basic input verification like empty string check, int == 0 , etc.

@ackleymi
Copy link
Contributor

@jacks821 and what is your thoughts on returning the day's OHLC values, instead of just the close?

@jacks821
Copy link
Contributor Author

I like that idea a lot. But I think i'd have to build it a little differently, unless you have an idea of how to do that using the Chart API? I'll work on just doing it separately.

@ackleymi
Copy link
Contributor

I'll work up an example and post it here in a bit

@jacks821
Copy link
Contributor Author

I fixed it so that it returns the OHLC values as just a ChartBar. I just wasn't understanding how the Iters worked.

@ackleymi
Copy link
Contributor

ackleymi commented Jan 8, 2019

@jacks821 this looks ok to merge. I'll tack on a follow up PR that adds gofmt shortcuts to the Makefile and make sure its listed in the requirements for subsequent PRs. I'll ping you that PR when I'm done to show you what I mean. :)

@jacks821
Copy link
Contributor Author

I don't mean to push you, just wanted to see if you were gonna show me those shortcuts? Also, i noticed you're from Ann Arbor. I'm originally from Detroit, myself.

@ackleymi ackleymi merged commit 35659a5 into piquette:master Jan 31, 2019
@ackleymi
Copy link
Contributor

@jacks821 whoops got sidetracked with another project, I merged it.

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.

None yet

3 participants