-
Notifications
You must be signed in to change notification settings - Fork 56
Refactor #39
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
Refactor #39
Conversation
@@ -33,7 +38,7 @@ class MyClass | |||
protected $httpClient; | |||
|
|||
/** | |||
* @param HttpClient|null $httpClient Client to do HTTP requests, if not set, autodiscovery will be used to find a HTTP client. | |||
* @param HttpClient|null $httpClient |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why remove the phpdoc explanation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it's an addition. The example code is pretty clean I think. I can add it back if you think it adds any value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great job! i commented on quite some places (and github did not see some of the moves, considering whole files as new. so i probably reviewed some text that you did not change at all) |
Thanks for the review. I applied the suggested changes, commented on a few. We will probably have to proofread the whole docs before release. |
great. i suggest we merge this and do further proofreading / review / improvements on the doc as people see things. this is a PR that should not stay open for long, or we risk annoying merge conflicts due to all the files moving around. |
Agree. |
Things worth to mention:
Please review as I cannot read any more docs and I am sure it is full of mistakes.