8000 fixed some deprecated notices by fabpot · Pull Request #13434 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

fixed some deprecated notices #13434

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 16, 2015
Merged

fixed some deprecated notices #13434

merged 1 commit into from
Jan 16, 2015

Conversation

fabpot
Copy link
Member
@fabpot fabpot commented Jan 16, 2015
Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? no
Fixed tickets -
License MIT
Doc PR -

@stof
Copy link
Member
stof commented Jan 16, 2015

👍

xabbuh
xabbuh reviewed Jan 16, 2015
View reviewed changes
src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/ExtensionPass.php
@@ -63,6 +64,10 @@ public function process(ContainerBuilder $container)
$container->getDefinition('twig.extension.code')->replaceArgument(0, $container->getParameter('templating.helper.code.file_link_format'));
}

if (!$container->has('security.token_storage')) {
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure? Shouldn't this be if ($container->has('security.token_storage')) (without the !)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm sure :) If the token storage does not exist, it means you are using an older version, where we need to inject the security service.

8000 Copy link
Member

Choose a reason for hiding this comment

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

Ah, now I get what you are trying to achieve. But then, the service definition requested via getDefinition() below should be twig.app_variable and not security.token_storage.

Copy link
Member Author

Choose a reason for hiding this comment

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

oops

@fabpot fabpot merged commit 84f3753 into symfony:2.7 Jan 16, 2015
fabpot added a commit that referenced this pull request Jan 16, 2015
@fabpot
minor #13434 fixed some deprecated notices (fabpot)
564ae34 
This PR was merged into the 2.7 branch.

Discussion
----------

fixed some deprecated notices

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

Commits
-------

84f3753 fixed some deprecated notices
fabpot added a commit to fabpot/symfony that referenced this pull request Jan 19, 2015
@fabpot
Revert "minor symfony#13434 fixed some deprecated notices (fabpot)"
1067cc5 
This reverts commit 564ae34, reversing
changes made to df76faa.
fabpot added a commit that referenced this pull request Jan 19, 2015
@fabpot
Revert "minor #13434 fixed some deprecated notices (fabpot)"
cbce4f1 
This reverts commit 564ae34, reversing
changes made to df76faa.
fabpot added a commit that referenced this pull request Jan 20, 2015
@fabpot
minor #13460 Fix usage of app.security (fabpot)
7ab2a56 
This PR was merged into the 2.7 branch.

Discussion
----------

Fix usage of app.security

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

Reverts the previous attempt to remove notices when using the `app.security` variable in Twig because it did not work (it prevents using `app.security` altogether).

So, instead, we inject the container and only get the security service (and thus triggering the deprecation notice) when the user is using `app.security`.

Commits
-------

09ed9b5 removed deprecated notices when using the security service
1067cc5 Revert "minor #13434 fixed some deprecated notices (fabpot)"
fabpot added a commit that referenced this pull request Jan 20, 2015
@fabpot
Merge branch '2.7'
30935c7 
* 2.7:
  removed deprecation notice for internal constant
  removed deprecated notices when using the security service
  [Validator] fixed deprecation notice for ElementMetadata
  Revert "minor #13434 fixed some deprecated notices (fabpot)"
  removed usage of the deprecated EsiListener class in core
  [security] Fetching current stored context when not explicitly specified
  [FrameworkBundle] Container parameters in Route#condition
  Deprecated setDefaultOptions() in favor of configureOptions()

Conflicts:
	src/Symfony/Bridge/Doctrine/composer.json
	src/Symfony/Bridge/Propel1/composer.json
	src/Symfony/Bridge/Twig/AppVariable.php
	src/Symfony/Bundle/FrameworkBundle/composer.json
	src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/CsrfFormLoginBundle/Form/UserLoginFormType.php
	src/Symfony/Component/Form/Tests/ResolvedFormTypeTest.php
@fabpot fabpot deleted the security-notices branch February 12, 2015 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fabpot @stof @xabbuh
0