Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[BrowserKit | WCM] added override power to server parameters provided on request method#9821
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
cordoval commentedDec 19, 2013
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #9527,#9762 |
| License | MIT |
| Doc PR | maybe if@wouterj give me a chance |
cordoval commentedDec 19, 2013
it chieffabpot commentedDec 19, 2013
Can you make a better commit message? We don't need to reference other PRs or issues there, we need to describe what the commit does. |
cordoval commentedDec 19, 2013
@fabpot ^^ 👶 |
fabpot commentedDec 19, 2013
The message should be something like "added ..." or "fixed ..." |
cordoval commentedDec 19, 2013
@fabpot ^^ 👶 |
fabpot commentedDec 19, 2013
As I said, |
cordoval commentedDec 19, 2013
@fabpot ^^ 👶 |
cordoval commentedDec 19, 2013
@fabpot do not merge yet, the check from the PR i did earlier on SE needs to be made compatible and this will adjust things on here |
cordoval commentedDec 19, 2013
@fabpot waiting only for travis now, all good and test from SE passes with this changes |
wouterj commentedDec 19, 2013
again, please don't ping me on each PR to let me say if docs are needed. You are old and wise enough to determine it yourself... |
cordoval commentedDec 19, 2013
@wouterj I spoke too soon, apologize, still working on the PR, and honestly back then I did not knew exactly |
fabpot commentedDec 23, 2013
Tests fail. |
cordoval commentedDec 23, 2013
cordoval commentedDec 24, 2013
@fabpot, tests are passing. 🎄 👶 |
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.
I would fix thegetAbsoluteUri() instead.
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.
what do you mean by fixinggetAbsoluteUri()? adding a new argument getAbsoluteUri($uri, $server) ? I think this is ok as-is but let me know.
fabpot commentedDec 27, 2013
As this is a bug fix, should be done on 2.3 instead. |
…re's no lib-intl (pamil)This PR was submitted for the 2.3-dev branch but it was merged into the 2.3 branch instead (closessymfony#9896).Discussion----------[Intl] Skip tests that need full lib-intl when there's no lib-intl| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |`NumberFormatter::formatCurrency` executes indirectly `Symfony\Component\Intl\ResourceBundle\Reader::read()`, which has `$bundle = new \ResourceBundle($locale, $path)` - there's Fatal Error if `\ResourceBundle` isn't found. Added availability to run unit tests if `lib-intl` isn't installed.Commits-------6614b66 Skips test that need full lib-intl.
cordoval commentedDec 30, 2013
closed in favor of#9901 |
This PR was merged into the 2.3 branch.Discussion----------Fixed server values in BrowserKit| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#9527,#9762,#9821,#9901| License | MIT| Doc PR | n/aCommits-------65b9810 fixed too greedy replacementsd9cf28d fixed protocol-relative URLs289da16 added override power to server parameters provided on request method