search

piquette / finance-go

:bar_chart: Financial markets data library implemented in go.
https://piquette.io/projects/finance-go
MIT License
724 stars 102 forks source link

added a historical quote function to Quote #11

Closed jacks821 closed 5 years ago

jacks821 commented 5 years ago

This only gives the closing price of the requested day.

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 275

Totals Coverage Status
Change from base Build 210: 0.2%
Covered Lines: 846
Relevant Lines: 997
💛 - Coveralls
ackleymi commented 5 years ago

@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 commented 5 years ago

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

ackleymi commented 5 years ago

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

ackleymi commented 5 years ago

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

jacks821 commented 5 years ago

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 commented 5 years ago

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

jacks821 commented 5 years ago

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

ackleymi commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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