SCA: minor code tweaks #30653
SCA: minor code tweaks #30653
Conversation
kalessil
commented
Mar 22, 2019
•
| Q | A |
|---|---|
| Branch? | 3.4 |
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | n/a |
| License | MIT |
| Doc PR | n/a |
- minor code tweaks
- drop private properties, which used as local variables
| $hasRun = &$this->hasTerminatedWithException; | ||
| $this->exceptionHandler = function (\Exception $e) use ($kernel, $request, &$hasRun) { | ||
| if ($hasRun) { | ||
| $this->exceptionHandler = function (\Exception $e) use ($kernel, $request) { |
There was a problem hiding this comment.
Another option here is to make the closure static. This would have the benefit of removing a circular reference.
There was a problem hiding this comment.
I like the proposal, would you consider enabling static_lambda CS fixer rule (I have to revert this specific change then)?
There was a problem hiding this comment.
please revert and add static instead yes
dunno about StaticLambdaFixer - why not :)
There was a problem hiding this comment.
Will do + will create a PR for static closures.
There was a problem hiding this comment.
BTW which branch to target with static closures, 3.4?
There was a problem hiding this comment.
BTW which branch to target with static closures, 3.4?
maybe none: the rule is risky and I'm not sure it makes sense to apply it globally now that I reviewed what it does :)
There was a problem hiding this comment.
Got it, but at least you are now aware of it =)
| @@ -78,11 +78,7 @@ private function getKernel() | |||
| ->expects($this->atLeastOnce()) | |||
| ->method('has') | |||
| ->will($this->returnCallback(function ($id) { | |||
| if ('console.command_loader' === $id) { | |||
There was a problem hiding this comment.
this applies to 3.4 also - maybe others too? they should be submitted against 3.4
There was a problem hiding this comment.
I'll re-check the PR changes against 3.4, thanks for reminding =)
|
Thank you @kalessil. |
This PR was merged into the 3.4 branch. Discussion ---------- SCA: minor code tweaks | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a - minor code tweaks - drop private properties, which used as local variables Commits ------- cc4529d SCA: minor code tweaks