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 stock summary retrieval and decoding functionality #48

Merged
merged 7 commits into from Mar 1, 2024

Conversation

elminson
Copy link
Contributor

Description
Adding stock Summary to get information about shares and stock data
I need this information to get the float shares of a stock
If I need this information maybe another one needs it

@scheb
Copy link
Owner

scheb commented Feb 25, 2024

Hi, could you please explain why this is needed. Isn't getQuote doing the same thing?

@elminson
Copy link
Contributor Author

Hi @scheb thank you for your quick response,
getQuote don't have some important information (in my case is critical to get the float shares, operatingCashflow, sharesShortPriorMonth, freeCashflow, etc).

I have 2 options,

  1. Create a new method (stockSummary)
  2. Add what I need to getQuote method.

Because was easier to tests and less disruptive to the current method, I had pick option 1

Let me know what do you think about it.

Here are the data for AAPL called by each method
stockSummary.json => https://gist.github.com/elminson/e996c7bbcd21c066406b651bb061775f

getQuote.json => https://gist.github.com/elminson/0d1e2afaba3b815de5cc06cc1fc1813c

@scheb
Copy link
Owner

scheb commented Feb 26, 2024

Okay, I see. Could you please do me a favor and rebase your changes onto the main branch and force-push an update? For some reason the CI didn't trigger. I hope it will after the push. I'm pretty sure there will be some code style issues popping up that we'd need to fix before we can merge.

@elminson elminson force-pushed the feature/addingStockSummary branch 2 times, most recently from 4f23158 to 877d188 Compare February 27, 2024 02:50
@elminson
Copy link
Contributor Author

@scheb I think now should be good if not let me know

@scheb
Copy link
Owner

scheb commented Feb 27, 2024

Please check the coding standards job, there have been some issues popping up.

@elminson
Copy link
Contributor Author

@scheb all code standards now are passing on my local please confirm :)

Repository owner deleted a comment from codecov-commenter Feb 28, 2024
Copy link
Owner

@scheb scheb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing, then we're good to go.

src/ApiClient.php Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered ☂️

@elminson
Copy link
Contributor Author

elminson commented Mar 1, 2024

fixing the failing standards
@scheb my PHP standard is set to use tabs :) fixed

@scheb scheb merged commit c24c085 into scheb:4.x Mar 1, 2024
5 checks passed
@scheb
Copy link
Owner

scheb commented Mar 1, 2024

Merged and tagged as v4.9.0

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