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

feat: Add request handler and a GapicRequestWrapper #6807

Conversation

saranshdhingra
Copy link
Contributor

@saranshdhingra saranshdhingra commented Nov 22, 2023

This PR adds the RequestHandler and the merged GapicRequestWrapper to the Core/ directory.

Note: Changed the target of this PR to another branch in googleapis.
Will open another PR using the newly created branch as the base branch, as discussed offline.

@saranshdhingra saranshdhingra marked this pull request as ready for review December 8, 2023 13:19
@saranshdhingra saranshdhingra requested review from a team as code owners December 8, 2023 13:19
Copy link
Contributor

@bshaffer bshaffer left a comment

Choose a reason for hiding this comment

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

I know we're waiting on the PubSub PR to reference how these classes will be used (which is pretty important to this review), but in the meantime we also need some tests to cover the new functionality as well.

Core/tests/Unit/Helpers/SampleGapicClass1.php Outdated Show resolved Hide resolved
* @internal
* Supplies helper methods to interact with the APIs.
*/
trait ApiHelpersTrait
Copy link
Contributor

Choose a reason for hiding this comment

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

We still need tests for this class!

Core/tests/Unit/RequestHandlerTest.php Outdated Show resolved Hide resolved
@saranshdhingra saranshdhingra changed the base branch from main to add-request-handler December 13, 2023 08:15
@saranshdhingra saranshdhingra merged commit ef252d7 into googleapis:add-request-handler Dec 13, 2023
20 checks passed
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.

2 participants