Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Add ControllerTrait::getParameter()#25439
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
chalasr commentedDec 11, 2017
| Q | A |
|---|---|
| Branch? | 4.1 |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #25288 (comment) |
| License | MIT |
| Doc PR | n/a |
96d41b7 to54f063eCompare| return; | ||
| } | ||
| if (method_exists($this,'expectException')) { |
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.
You can remove this check for new code. As Symfony 4 requires PHP 7.1+, it will not need to run tests on PHPUnit 4.8 (which is needed for PHP 5.5 and older). SoexpectException can always be used.
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.
thanks, we have a bunch of similar checks to removehttps://github.com/symfony/symfony/search?utf8=%E2%9C%93&q=setExpectedException&type=
54f063e to799bc10Compare8d047c9 to2262186Comparenicolas-grekas commentedDec 11, 2017
should we add an |
chalasr commentedDec 11, 2017
@nicolas-grekas |
| * | ||
| * @return mixed | ||
| * | ||
| * @final since version 4.1 |
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.
just@final (since always - it didn't exist before)
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.
fixed
nicolas-grekas left a comment
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.
ok without hasParameter, since it doesn't exist on Controller
2262186 to28397e5Comparefabpot commentedDec 11, 2017
Thank you@chalasr. |
This PR was merged into the 4.1-dev branch.Discussion----------Add ControllerTrait::getParameter()| Q | A| ------------- | ---| Branch? | 4.1| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#25288 (comment)| License | MIT| Doc PR | n/aCommits-------28397e5 Add ControllerTrait::getParameter()
curry684 commentedMay 29, 2018
I just tried using this because oftoday's blog post and it seems quite fundamentally broken? The |
curry684 commentedMay 29, 2018
Oh it got screwed over later at3051289#diff-ef10778bc68793f8c8f4b71a7ba67790R86 while refactoring,@nicolas-grekas missed 3 characters in typing it hehe. |
… (curry684)This PR was submitted for the master branch but it was squashed and merged into the 4.1 branch instead (closes#27415).Discussion----------Insert correct parameter_bag service in AbstractControllerReverts this feature being broken in3051289#diff-ef10778bc68793f8c8f4b71a7ba67790R86 - `getParameter` could never work now as it was querying the container itself instead of the parameter bag.| Q | A| ------------- | ---| Branch? | 4.1| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes <!-- please add some, will be required by reviewers -->| License | MITAlso see comments at#25439 (comment)Commits-------37270d7 Insert correct parameter_bag service in AbstractController