Skip to content

[HTTP Foundation] Deprecate passing argument to method Request::isMethodSafe() #31658

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

Merged
merged 1 commit into from
Jun 5, 2019

Conversation

dFayet
Copy link

@dFayet dFayet commented May 28, 2019

Q A
Branch? 4.2
Bug fix? no
New feature? no
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets #31323
License MIT
Doc PR

Passing argument to Request::isMethodSafe() should have been deprecated in 4.1. As mentionned there: https://github.com/symfony/http-foundation/blob/master/Request.php#L1435-L1452

We also remove Exceptions throwed when you call Request::isMethodSafe() or Request::isMethodSafe(true)

@dFayet dFayet force-pushed the hotfix/issue-31323 branch 2 times, most recently from de78c59 to d3e0689 Compare May 28, 2019 16:44
@dFayet dFayet changed the title Deprecate passing argument to method Request::isMethodSafe() [HTTP Foundation] Deprecate passing argument to method Request::isMethodSafe() May 29, 2019
@nicolas-grekas nicolas-grekas added this to the next milestone May 29, 2019
@nicolas-grekas
Copy link
Member

Thanks, please rebase and retarget for branch 4.4, as we don't add deprecations on other branches.

@dFayet dFayet changed the base branch from 4.2 to 4.4 May 29, 2019 14:41
@dFayet
Copy link
Author

dFayet commented May 29, 2019

@nicolas-grekas Done
Even if I'm not sure of what I have done :/

@fabpot
Copy link
Member

fabpot commented Jun 4, 2019

@dFayet It looks like your merged a branch here. It makes the pull request not-mergeable on our side. Can you rebase your branch (it will remove the merge commit)?

@dFayet dFayet force-pushed the hotfix/issue-31323 branch from 3a4321d to 6aa78cf Compare June 4, 2019 09:04
@dFayet
Copy link
Author

dFayet commented Jun 4, 2019

@fabpot done

@fabpot
Copy link
Member

fabpot commented Jun 4, 2019

Can you have a look at the tests, they seem to be broken because of these changes (low).

@dFayet
Copy link
Author

dFayet commented Jun 4, 2019

@fabpot Done for low. Is there anything to be done for the high one?

@fabpot fabpot force-pushed the hotfix/issue-31323 branch from 7fc0df1 to 59fa1bd Compare June 5, 2019 01:45
@fabpot
Copy link
Member

fabpot commented Jun 5, 2019

Thank you @dFayet.

@fabpot fabpot merged commit 59fa1bd into symfony:4.4 Jun 5, 2019
fabpot added a commit that referenced this pull request Jun 5, 2019
… Request::isMethodSafe() (dFayet)

This PR was squashed before being merged into the 4.4 branch (closes #31658).

Discussion
----------

[HTTP Foundation] Deprecate passing argument to method Request::isMethodSafe()

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #31323    <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        |  <!-- required for new features -->

Passing argument to `Request::isMethodSafe()` should have been deprecated in 4.1. As mentionned there:  https://github.com/symfony/http-foundation/blob/master/Request.php#L1435-L1452

We also remove Exceptions throwed when you call `Request::isMethodSafe()`  or `Request::isMethodSafe(true)`

Commits
-------

59fa1bd [HTTP Foundation] Deprecate passing argument to method Request::isMethodSafe()
@nicolas-grekas nicolas-grekas removed this from the next milestone Oct 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants