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

[Request] Added a getBaseServerUrl() function #4312

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@COil
Contributor

COil commented May 17, 2012

This a clean PR related to #3416

(old PR should be closed)

Bug fix: [no]
Feature addition: [yes]
Backwards compatibility break: [yes]
Symfony2 tests pass: [yes]
Fixes the following tickets: [#3416, #3056]
Todo: [doc ?]
License of the code: MIT
Documentation PR: []

@travisbot

This comment has been minimized.

Show comment
Hide comment
@travisbot

travisbot May 17, 2012

This pull request passes (merged b7e63cb into e351c9f).

travisbot commented May 17, 2012

This pull request passes (merged b7e63cb into e351c9f).

@Tobion

This comment has been minimized.

Show comment
Hide comment
@Tobion

Tobion May 17, 2012

Member

The word server has nothing to do with the request. So the name is not good.
In terms of the URI spec it should probably be named getSchemeAuthority.

Member

Tobion commented May 17, 2012

The word server has nothing to do with the request. So the name is not good.
In terms of the URI spec it should probably be named getSchemeAuthority.

@COil

This comment has been minimized.

Show comment
Hide comment
@COil

COil May 24, 2012

Contributor

@Tobion getSchemeAuthority sounds quite complicated to me. @stof ?

Contributor

COil commented May 24, 2012

@Tobion getSchemeAuthority sounds quite complicated to me. @stof ?

@COil

This comment has been minimized.

Show comment
Hide comment
@COil

COil May 26, 2012

Contributor

If we want to be logical with the actual request class, if should be named: getSchemeAndHost

Contributor

COil commented May 26, 2012

If we want to be logical with the actual request class, if should be named: getSchemeAndHost

@COil

This comment has been minimized.

Show comment
Hide comment
@COil

COil Jun 7, 2012

Contributor

Ping ! :)

Contributor

COil commented Jun 7, 2012

Ping ! :)

@fabpot fabpot closed this in df8d94e Jun 28, 2012

fabpot added a commit that referenced this pull request Jun 28, 2012

merged branch fabpot/request-methods (PR #4679)
Commits
-------

df8d94e added Request::getSchemeAndHttpHost() and Request::getUserInfo() (closes #4312, refs #3416, refs #3056)

Discussion
----------

added Request::getSchemeAndHttpHost() and Request::getUserInfo()

see #4312

---------------------------------------------------------------------------

by travisbot at 2012-06-28T15:22:03Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1730172) (merged 598bd56f into 0d27570).

---------------------------------------------------------------------------

by Seldaek at 2012-06-28T15:22:35Z

Why not just `getSchemeAndHost`? That sounds long enough, and is fairly explicit given the context.

---------------------------------------------------------------------------

by fabpot at 2012-06-28T15:25:34Z

@Seldaek because (and that's probably unfortunate) we have both `getHost()` and `getHttpHost()`. The former does not include the port whereas the latter does.

---------------------------------------------------------------------------

by Seldaek at 2012-06-28T15:26:42Z

Ok makes sense.

---------------------------------------------------------------------------

by travisbot at 2012-06-28T16:11:16Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1730630) (merged df8d94e into 884a826).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment