Conversation
soullivaneuh
force-pushed
the
request-uri-manipulations
branch
from
c29bfde to
7df95c9
Compare
dbu
left a comment
dbu
left a comment
There was a problem hiding this comment.
we should add a usage example for each plugin, not only the BaseUriPlugin. but i can do that later once this PR is merged - the AddHostPlugin doc was completely missing before, so this is already an improvement.
|
|
||
| $plugin = new BaseUriPlugin(UriFactoryDiscovery::find()->createUri('https://domain.com:8000/api'), [ | ||
| // Always replace the host, even if this one is provided on the sent request. Available for AddHostPlugin. | ||
| 'replace' => 'true', |
There was a problem hiding this comment.
afaik the value here should be boolean true, not string
|
|
||
| Request URI manipulations can be done thanks to several plugins: | ||
|
|
||
| * ``AddHostPlugin``: Sets default host, scheme and port of each request. |
There was a problem hiding this comment.
depending on configuration, this can force the host or set it to a default value. maybe "Set host, scheme and port. Depending on configuration, the host is overwritten in every request or only set if not yet defined in the request."
| Request URI manipulations can be done thanks to several plugins: | ||
|
|
||
| * ``AddHostPlugin``: Sets default host, scheme and port of each request. | ||
| * ``AddPathPlugin``: Sets default base path of each request. |
There was a problem hiding this comment.
this is not a default but unconditionally applied: "Prefix the request path with a path, leaving the host information untouched."
|
|
||
| * ``AddHostPlugin``: Sets default host, scheme and port of each request. | ||
| * ``AddPathPlugin``: Sets default base path of each request. | ||
| * ``BaseUriPlugin``: Is a combination of ``AddHostPlugin`` and ``AddPathPlugin`` using the ``UriInterface`` class. |
There was a problem hiding this comment.
lets leave away the mention of UriInterface, that is just an implementation detail
|
Thank you for this PR. Could you fix the issues David commented? I would be happy to improve the documentation with this PR. |
|
Hello @Nyholm. I was on vacation. I'll work on it today or next week! 👍 |
soullivaneuh
force-pushed
the
request-uri-manipulations
branch
from
7df95c9 to
364f596
Compare
|
All requested changes are done @dbu. This PR is ready for review and merge. |
|
Thanks @soullivaneuh |
dbu
left a comment
dbu
left a comment
There was a problem hiding this comment.
found one formatting error, otherwise its all fine
| Request URI manipulations can be done thanks to several plugins: | ||
|
|
||
| * ``AddHostPlugin``: Set host, scheme and port. Depending on configuration, | ||
| the host is overwritten in every request or only set if not yet defined in the request. |
There was a problem hiding this comment.
if you want to have multiline in a bullet list, you need to indent the second line by two spaces to align with the ` above.
There was a problem hiding this comment.
This looks to be related to the failing job.
|
Ah, build is failing. Can you please check? https://travis-ci.org/php-http/documentation/builds/173191035#L333 |
soullivaneuh
force-pushed
the
request-uri-manipulations
branch
from
364f596 to
ea803b0
Compare
|
It should be OK now. |
|
Build passed! 🍾 |
4 participants
Closes #162
Related source PR: php-http/client-common#48