Skip to content

[2.1][TwigBridge] Fixes Issue #7342 in TwigBridge #7344

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

Closed
wants to merge 3 commits into from
Closed

[2.1][TwigBridge] Fixes Issue #7342 in TwigBridge #7344

wants to merge 3 commits into from

Conversation

benbender
Copy link

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

@stof
Copy link
Member

stof commented Mar 12, 2013

Can you add a test to avoid regressions ?

@benbender
Copy link
Author

Done

@stevelacey
Copy link

Looks good to me?

$result = $scope->get('test');
$scope->leave();

$this->assertEquals($result, null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could simplify this:

$scope = new Scope();
$scope->enter();

$this->assertNull($scope->get('test'));

@pborreli
Copy link
Contributor

👍

fabpot added a commit that referenced this pull request Mar 13, 2013
This PR was squashed before being merged into the 2.1 branch (closes #7344).

Commits
-------

c423f16 [2.1][TwigBridge] Fixes Issue #7342 in TwigBridge

Discussion
----------

[2.1][TwigBridge] Fixes Issue #7342 in TwigBridge

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7342
| License       | MIT
| Doc PR        | --

---------------------------------------------------------------------------

by stof at 2013-03-12T13:28:15Z

Can you add a test to avoid regressions ?

---------------------------------------------------------------------------

by benbender at 2013-03-12T13:54:02Z

Done

---------------------------------------------------------------------------

by stevelacey at 2013-03-12T14:40:59Z

Looks good to me?

---------------------------------------------------------------------------

by pborreli at 2013-03-12T16:04:27Z

:+1:
@fabpot fabpot closed this Mar 13, 2013
@benbender
Copy link
Author

The bug originally fixed by this PR is now double fixed. This PR should be reverted.

fabpot added a commit that referenced this pull request Mar 15, 2013
* 2.1:
  sub-requests are now created with the same class as their parent
  [FrameworkBundle] removed BC break
  [FrameworkBundle] changed temp kernel name in cache:clear
  [DoctrineBridge] Avoids blob values to be logged by doctrine
  [Security] use current request attributes to generate redirect url?
  [Validator] fix showing wrong max file size for upload errors
  [TwigBridge] removed double var initialization (refs #7344)
  [2.1][TwigBridge] Fixes Issue #7342 in TwigBridge
  [FrameworkBundle] fixed cahe:clear command's warmup
  [TwigBridge] now enter/leave scope on Twig_Node_Module
  [TwigBridge] fixed fixed scope & trans_default_domain node visitor
  [TwigBridge] fixed non probant tests & added new one
  [BrowserKit] added ability to ignored malformed set-cookie header
  [Translation] removed wriong 'use'
  [Translation] added xliff loader/dumper with resname support
  [TwigBridge] fixes

Conflicts:
	src/Symfony/Bundle/FrameworkBundle/HttpKernel.php
	src/Symfony/Component/Security/Http/HttpUtils.php
	src/Symfony/Component/Translation/Loader/XliffFileLoader.php
	src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants