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

Update docblock for APIException class' constructor function to include correct params and param types #1

Open
sampurcell opened this issue Dec 3, 2018 · 4 comments

Comments

@sampurcell
Copy link

@VicV came across a minor issue in the SDK, its throwing an error in our IDE, when you have the time would appreciate an update. Thanks!

Description

The APIException::_construct method parameters are string $reason and TaplyticsLib\Http\HttpContext $context however the docblock refers to an int $responseCode and string $responseBody instead which raises a warning in the IDE when mocking the object for tests, etc.

Link to the docblock: APIException.php#L31-L33

@VicV
Copy link
Contributor

VicV commented Dec 4, 2018

Hey @sampurcell sorry for the delayed response! I'll take a look at this shortly.

@kavitasookrah
Copy link

Hey @sampurcell How's it going? Just following up here - Can you provide a bit more context around the error you're seeing and what you're hoping to do. Let me know if this is blocking you in anyway.

@sampurcell
Copy link
Author

Hi, no problem.

It is not blocking and not really an error, just a warning in my IDE. Here are some more details:

When I mock an APIException, I get a warning that looks this this:

screen shot 2018-12-04 at 2 24 09 pm

Also in the APIException class I also see this warning:

screen shot 2018-12-04 at 3 02 28 pm

The reason it gives this warning seems to be because the docblock for APIException defines the params as a string $reason, int $responseCode and string $responseBody

However if you look at the APIException constructor, you'll see it actually wants a string $reason and an HttpContext $context.

So it thinks the second parameter is an int instead of an HttpContext instance and gives the warnings seen in the screenshots above.

To fix, I think you just need to add:
use TaplyticsLib\Http\HttpContext; at the top of the APIException.php file and repalce:

* @param int $responseCode the HTTP response code from the API request
* @param string $responseBody the HTTP response body from the API request

In the APIException constructor docblock with:
@param HttpContext $context

Should quiet the warnings when creating instances of the class in the IDE.

@kavitasookrah
Copy link

Hey @sampurcell this is great! Thanks very much for all the details. I'll pass this on to the team and get you an update :)

jsalaber added a commit that referenced this issue Apr 5, 2019
* Added feature flags

* added circle config
jsalaber added a commit that referenced this issue Apr 5, 2019
[Feature] Added feature flags (#1)

* Added feature flags

* added circle config

Fixed description

fixed circle yml file

added status badge and updated circle to have names for commands

fixed indenting

updated composer.json file

renamed getting started file

small doc change (#2)

* small doc change

* removed print log and added comment for method
jsalaber pushed a commit that referenced this issue Apr 5, 2019
[Feature] Added feature flags (#1)

* Added feature flags

* added circle config

Fixed description

fixed circle yml file

added status badge and updated circle to have names for commands

fixed indenting

updated composer.json file

renamed getting started file

small doc change (#2)

* small doc change

* removed print log and added comment for method
jsalaber added a commit that referenced this issue Apr 5, 2019
* Added feature flags

[Feature] Added feature flags (#1)

* Added feature flags

* added circle config

Fixed description

fixed circle yml file

added status badge and updated circle to have names for commands

fixed indenting

updated composer.json file

renamed getting started file

small doc change (#2)

* small doc change

* removed print log and added comment for method

* added some clarity for format of event body
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants