Skip to content

Update Client.php #26508

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Update Client.php #26508

wants to merge 3 commits into from

Conversation

jrean
Copy link
Contributor

@jrean jrean commented Mar 13, 2018

Docblock.

@jrean
Update Client.php
6a383ff 
- Docblock
- Coding standard from Fabbot
fabpot
fabpot requested changes Mar 13, 2018
View reviewed changes
src/Symfony/Component/BrowserKit/Client.php Outdated
@@ -365,7 +365,7 @@ protected function doRequestInProcess($request)
unlink($deprecationsFile);
foreach ($deprecations ? unserialize($deprecations) : array() as $deprecation) {
if ($deprecation[0]) {
trigger_error($deprecation[1], E_USER_DEPRECATED);
@trigger_error($deprecation[1], E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

This one should be reverted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted it (your fabbot was asking to add the @)

src/Symfony/Component/BrowserKit/Client.php Outdated
@@ -606,6 +606,14 @@ protected function requestFromRequest(Request $request, $changeHistory = true)
return $this->request($request->getMethod(), $request->getUri(), $request->getParameters(), $request->getFiles(), $request->getServer(), $request->getContent(), $changeHistory);
}

/**
* Update the server setting from the given URI.
Copy link
Member

Choose a reason for hiding this comment

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

Updates

Copy link
Member

Choose a reason for hiding this comment

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

Oh, that's a private method, we don't document private method except when really needed. So, this docblock and the next should be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't notice! Sorry. I reverted.

src/Symfony/Component/BrowserKit/Client.php Outdated
@@ -616,6 +624,13 @@ private function updateServerFromUri($server, $uri)
return $server;
}

/**
* Extract host value from the URI.
Copy link
Member

Choose a reason for hiding this comment

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

Extracts

@jrean
Update Client.php
be3e51c 
Revert unnecessary changes
@fabpot
Copy link
Member

fabpot commented Mar 13, 2018

Thank you @jrean.

fabpot added a commit that referenced this pull request Mar 13, 2018
@fabpot
minor #26508 Update Client.php (jrean)
985ef53 
This PR was submitted for the master branch but it was squashed and merged into the 3.4 branch instead (closes #26508).

Discussion
----------

Update Client.php

Docblock.

Commits
-------

5d7b131 Update Client.php
@fabpot fabpot closed this Mar 13, 2018
@xabbuh xabbuh added this to the 3.4 milestone Apr 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@derrabus derrabus derrabus approved these changes

Assignees
No one assigned
Labels
Status: Reviewed
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
5 participants
@jrean @fabpot @derrabus @xabbuh @carsonbot