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

Add an option to disable fetch_31() during init Stock #31

Merged
merged 2 commits into from
Mar 12, 2018

Conversation

ianlini
Copy link
Contributor

@ianlini ianlini commented Mar 9, 2018

  1. It's a bad idea to send requests in __init__()
  2. fetch_31() is a strange method and it sends too many requests (3 in the worst case)
  3. In most cases, I want to get the data in a specific month or date, so I don't need the initial fetch_31() and want to fully control what requests I will send

Therefore, I add an option to disable the initial fetch_31(), and leave the default behavior the same as in the original implementation.

@mlouielu
Copy link
Owner

mlouielu commented Mar 9, 2018

Thanks for your contribution @ianlini, could you please also help to update the documentation in docs?

@ianlini
Copy link
Contributor Author

ianlini commented Mar 9, 2018

Sure. I will try.

@ianlini
Copy link
Contributor Author

ianlini commented Mar 11, 2018

Did I update the docs correctly?

@mlouielu
Copy link
Owner

Hi @ianlini , I'm on a break yesterday. Yes, you update the code correctly, I'll accept this commit.

Another thing is, please let your commit message have a capital letter in the first words! Consistent is good for our code!

Many thanks for your contribution, @ianlini .

@mlouielu mlouielu merged commit 3752980 into mlouielu:master Mar 12, 2018
@ianlini ianlini changed the title add an option to disable fetch_31() during init Stock Add an option to disable fetch_31() during init Stock Mar 12, 2018
@mlouielu
Copy link
Owner

mlouielu commented Mar 12, 2018

@ianlini, commit message and issue title are different on GitHub, you should also check your commit message in your local git.

If you want to change commit message in your latest commit, use git commit --amend, if you want to change a old commit message, you may need to checkout git rebase -i <your target commit hash>.

ref: https://git-scm.com/book/zh-tw/v1/Git-%E5%B7%A5%E5%85%B7-%E9%87%8D%E5%AF%AB%E6%AD%B7%E5%8F%B2

@ianlini
Copy link
Contributor Author

ianlini commented Mar 12, 2018

I know. Just want to change the title first.
The commit message is actually the title of the PR in your current setting, so I think I won't change the commit I have pushed.

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

2 participants