-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Security] Split the component into 3 sub-components Core, ACL, HTTP #9047
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
Conversation
@@ -0,0 +1,19 @@ | |||
Copyright (c) 2004-2013 Fabien Potencier |
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.
put the LICENSE under a meta folder according to best practices
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.
Nope, that's only for bundles.
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.
oh ok sorry did not know, pardon me sil vou plait 👶
I'm definitely in favor of this split... if we can do it without BC breaks... so the old class names must still exist. By the way, I think it would make #8848 obsolete. |
You can run the unit tests with the following command: | ||
|
||
$ cd path/to/Symfony/Component/Security/Http/ | ||
$ composer.phar install --dev |
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.
same here --dev
@fabpot The only class names that were changed so far are the tests, where the namespaces changed from I'm not quite sure what to do about the |
@fabpot There is no BC breaks. Only test classes are renamed in this PR. All other classes are still the same. for the resources, I think we should find a way to have them in one of the components. Having to require the |
@stof Well one possibility would be to move them to wherever they belong (Core or HTTP?), and to add a Composer post-install hook that copies them to the main directory in |
I suppose that the goal is to move the classes themselves as well, right? |
@bschussek a post-install hook is not a good BC layer as it has to be added by the user in the root composer.json |
@fabpot The Security component is already divided in 3 distinct namespaces. This PR simply creates the subtree splits of these 3 folders. |
... I need to sleep more ;) |
For the Resources directory, I would copy to Core (as Http depends on Core), deprecate the old location, and remove it in 3.0. In the meantime, we must ensure that both Resources directory are kept in sync. |
Closing in favor of #9064 |
This PR was merged into the master branch. Discussion ---------- [Security] Split the component into 3 sub-components Core, ACL, HTTP | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #9047, #8848 | License | MIT | Doc PR | - The rationale behind this PR is to be able to use any of the sub components without requiring all the dependencies of the other sub components. Specifically, I'd like to use the core utils for an improved CSRF protection mechanism (#6554). Commits ------- 14e9f46 [Security] removed unneeded hard dependencies in Core 5dbec8a [Security] fixed README files 62bda79 [Security] copied the Resources/ directory to Core/Resources/ 7826781 [Security] Split the component into 3 sub-components Core, ACL, HTTP
The rationale behind this PR is to be able to use any of the sub components without requiring all the dependencies of the other sub components. Specifically, I'd like to use the core utils for an improved CSRF protection mechanism (#6554).
TODO: