search

mathpaquette / IQFeed.CSharpApiClient

IQFeed.CSharpApiClient is fastest and the most well-designed C# DTN IQFeed socket API connector available
MIT License
121 stars 42 forks source link

New Lookup APIs with tests & other bug fixes #46

Closed mrut2pac closed 5 years ago

mrut2pac commented 5 years ago
  1. Added lookup APIs
  2. Added unit tests for lookup APIs
  3. Added integration tests for lookup APIs
  4. Some bug fixes
  5. Some failing tests fixes
mathpaquette commented 5 years ago

awesome! ill add my comments very soon. thanks!

mathpaquette commented 5 years ago

@mrut2pac I can't merge the PR as is because of the unnecessary merge conflict. Ill give you the procedure to recreate it as it should be.

mathpaquette commented 5 years ago

@mrut2pac Make sure you have my repo as one of your remotes: git remote -v

If you don't see https://github.com/mathpaquette/IQFeed.CSharpApiClient.git add it as the upstream: git remote add upstream https://github.com/mathpaquette/IQFeed.CSharpApiClient.git

git fetch upstream git checkout -b "feat-symbol" "origin/master" git cherry-pick --no-commit 0f8aa56880283fd3700c84ba18d1e7f0aabc06a2 git cherry-pick --no-commit 10095159b4ee80ff1baa1b2c133ac4d00011b23c git cherry-pick --no-commit 48cb0d268158d73a6a2ebed8843ea2db288b6d83 git cherry-pick --no-commit b19d10735370cfc84c249a04d78d69d9957e28e0 git add * git commit -m "feat(lookup/symbol): add support for symbol lookup"

recreate the PR from the feat-symbol branch

thanks

mathpaquette commented 5 years ago

@mrut2pac you got my comment on the PR?

mrut2pac commented 5 years ago

Yeah. I was actually hoping to get some change suggestions cause I noticed I misspelled NAICS in my changes and wanted to fix it :-) If you don’t want any other changes I will fix these in my master, then follow your steps to create a new Pull Request.

Regards, Vardan

On Oct 2, 2019, at 3:38 PM, Mathieu Paquette notifications@github.com wrote:

 @mrut2pac you got my comment on the PR?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

mathpaquette commented 5 years ago

was a bit not practical to start reviewing in because too much files were changed. some slight changes might be required but we can talk about it later.

mrut2pac commented 5 years ago

I guess I need to do git checkout -b "feat-symbol" "upstream/master" instead or "origin/master"

mrut2pac commented 5 years ago

also added git cherry-pick --no-commit 97979cc (my latest commit)

mathpaquette commented 5 years ago

I guess I need to do git checkout -b "feat-symbol" "upstream/master" instead or "origin/master"

true

mrut2pac commented 5 years ago

Updated the PR, please have a look

mathpaquette commented 5 years ago

sure, doing my very last pass before merging 🙂

thanks for all your efforts. this is very much appreciated and will help lots of people

From: mrut2pac notifications@github.com Sent: Friday, October 4, 2019 11:06 PM To: mathpaquette/IQFeed.CSharpApiClient IQFeed.CSharpApiClient@noreply.github.com Cc: Mathieu Paquette me@mathpaquette.com; State change state_change@noreply.github.com Subject: Re: [mathpaquette/IQFeed.CSharpApiClient] New Lookup APIs with tests & other bug fixes (#46)

Updated the PR, please have a look

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://github.com/mathpaquette/IQFeed.CSharpApiClient/pull/46?email_source=notifications&email_token=ABBICN7655LCKRGA4QECELDQNAAB5A5CNFSM4I3TQ6JKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEANI2CA#issuecomment-538610952, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABBICN2WQLCYWP4LX57KW4TQNAAB5ANCNFSM4I3TQ6JA.

mrut2pac commented 5 years ago

Thank you for starting it, this already benefits lot of people! :-)

Regards, Vardan

On Oct 5, 2019, at 6:49 AM, Mathieu Paquette notifications@github.com wrote:

sure, doing my very last pass before merging 🙂

thanks for all your efforts. this is very much appreciated and will help lots of people

From: mrut2pac notifications@github.com Sent: Friday, October 4, 2019 11:06 PM To: mathpaquette/IQFeed.CSharpApiClient IQFeed.CSharpApiClient@noreply.github.com Cc: Mathieu Paquette me@mathpaquette.com; State change state_change@noreply.github.com Subject: Re: [mathpaquette/IQFeed.CSharpApiClient] New Lookup APIs with tests & other bug fixes (#46)

Updated the PR, please have a look

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://github.com/mathpaquette/IQFeed.CSharpApiClient/pull/46?email_source=notifications&email_token=ABBICN7655LCKRGA4QECELDQNAAB5A5CNFSM4I3TQ6JKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEANI2CA#issuecomment-538610952, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABBICN2WQLCYWP4LX57KW4TQNAAB5ANCNFSM4I3TQ6JA. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.