Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[TwigBridge] Fix namespaced classes#23073
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
ogizanagi commentedJun 5, 2017
| Q | A |
|---|---|
| Branch? | 2.7 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #23072 |
| License | MIT |
| Doc PR | N/A |
alOneh commentedJun 5, 2017
I do the same fix, too late... ;) |
GromNaN commentedJun 5, 2017
This PR was merged into the 1.x branch.Discussion----------Use class_exists instead of requireI think this is required forsymfony/symfony#23073 to pass. It's probably very similar tosymfony/symfony#22657.A simple reproducer:```php<?phpuse Twig\Node\Node;require_once __DIR__ . '/vendor/autoload.php';new Node();new \Twig_Node();```[Without this patch](https://travis-ci.org/symfony/symfony/jobs/239712676#L2100):```phpPHP Fatal error: Cannot declare class Twig_Node, because the name is already in use in vendor/twig/twig/lib/Twig/Node.php on line 20```With, everything works fine, whatever the two calls order.Commits-------2c174e4 Use class_exists instead of require
weaverryan commentedJun 6, 2017
Was this the only file that had an issue? It's the only one that caused issues for me, but I wasn't sure if possibly some bad find/replace caused issues in other files. And yea... it's not technically needed, but it would be awesome if the test was extended to catch this - it's surprising to see an error like this not caught by a test. And mostly, thanks for the fast PR :) |
fabpot commentedJun 6, 2017
Thank you@ogizanagi. |
This PR was merged into the 2.7 branch.Discussion----------[TwigBridge] Fix namespaced classes| Q | A| ------------- | ---| Branch? | 2.7 <!-- see comment below -->| Bug fix? | yes| New feature? | no <!-- don't forget updating src/**/CHANGELOG.md files -->| BC breaks? | no| Deprecations? | no <!-- don't forget updating UPGRADE-*.md files -->| Tests pass? | yes| Fixed tickets |#23072 <!-- #-prefixed issue number(s), if any -->| License | MIT| Doc PR | N/ACommits-------a1cdc2d [TwigBridge] Fix namespaced classes
ogizanagi commentedJun 6, 2017
@weaverryan : I confirm. I did an inspection for other class not found occurrences in the bridge and the bundle but didn't find anything else. |