Skip to content
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

[RFC] Move annotations from SensioFrameworkExtraBundle to Symfony core #25361

Closed
javiereguiluz opened this issue Dec 6, 2017 · 16 comments
Closed
Labels
Keep open RFC RFC = Request For Comments (proposals about features that you want to be discussed)

Comments

@javiereguiluz
Copy link
Member

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? yes
Symfony version 4.1

SensioFrameworkExtraBundle provides three main features:

  • Annotations (@Route(), @Security(), ...)
  • Param converters
  • PSR-7

This proposal is only about annotations. We started moving some annotations into Symfony core, but we only moved one of them and that's why now we have two @Route annotations:

use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Symfony\Component\Routing\Annotation\Route;

So, should we move the rest of annotations to avoid mixing namespaces like this?

use Symfony\Component\Routing\Annotation\Route;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;

If you agree with this, should we move each annotation to the related package?

use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\HttpFoundation\Annotation\Method;
use Symfony\Component\Security\Annotation\Security;
// ...

Or should we put all annotations in the same place to make things simple?

use Symfony\Bridge\Annotation\Route;
use Symfony\Bridge\Annotation\Method;
use Symfony\Bridge\Annotation\Security;
// ...
@javiereguiluz javiereguiluz added the RFC RFC = Request For Comments (proposals about features that you want to be discussed) label Dec 6, 2017
@nicolas-grekas
Copy link
Member

Duplicate of #25103

@nicolas-grekas nicolas-grekas marked this as a duplicate of #25103 Dec 6, 2017
@javiereguiluz
Copy link
Member Author

Not entirely 😄 The other PR only talks about routing. Here we are talking about all annotations.

@bitgandtter
Copy link
Contributor

I really like the idea of moving annotations and param converters to respective packages or a bridge on core, its a really common and advised practice to use it and core should own it

@curry684
Copy link
Contributor

curry684 commented Dec 6, 2017

I'm with @bitgandtter on this one - I've never used routing annotations (I don't like my routing to be spread over 100+ files and their annotations) but I do use ParamConverters wherever possible, and the security annotations on class levels as extra guard. As far as I'm concerned these things are and should be core.

@mateuszsip
Copy link
Contributor

Great idea.
For me annotations should be moved to related components.

@garak
Copy link
Contributor

garak commented Dec 10, 2017

Why in the core? IMHO a dedicated component (or bridge/bundle) would be better

@bitgandtter
Copy link
Contributor

I agree with @garak but only if this new bridge or bundle is set as a dependency of the FrameworkBundle

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Dec 10, 2017

This is not going to happen: splitting is a significant effort. Unless you want to make it happen...
Either it's in core and this makes the life of the core team easier while making the annotation more widespread, or this stays here IMHO.

javiereguiluz added a commit to symfony/demo that referenced this issue Dec 11, 2017
This PR was merged into the master branch.

Discussion
----------

Use the Route annotation from Symfony core

Related to symfony/symfony#25361.

Commits
-------

f35cddb Use the Route annotation from Symfony core
@javiereguiluz javiereguiluz added this to TODO in Help Wanted Jul 30, 2019
@javiereguiluz javiereguiluz added this to Javier Wishlist in Help Wanted Jul 30, 2019
@javiereguiluz javiereguiluz added this to Javier's in Help Wanted Jul 30, 2019
@javiereguiluz
Copy link
Member Author

Closing because thanks to the new PHP 8 attributes, we shouldn't focus on PHPdoc-based annotations but on creating new attributes.

@bitgandtter
Copy link
Contributor

It should be the same, annotations, or attributes the main idea of this issue its to not depend on framework extra bundle for these common and high used features. IMO this issue it's still valid, maybe change the name but the scope it's still true

@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@carsonbot
Copy link

Just a quick reminder to make a comment on this. If I don't hear anything I'll close this.

@derrabus
Copy link
Member

Leave it open, please.

@ro0NL
Copy link
Contributor

ro0NL commented Jun 14, 2021

Meanwhile sensiolabs/SensioFrameworkExtraBundle#707

at this point maybe just close along with #40571, and take a per-concept approach

@javiereguiluz javiereguiluz removed this from Javier's in Help Wanted Oct 29, 2021
@javiereguiluz
Copy link
Member Author

Closing in favor of #44705.

@derrabus
Copy link
Member

Oh right, there was already an issue open. 😅

sayjun0505 added a commit to sayjun0505/sym_proj that referenced this issue Apr 16, 2023
This PR was merged into the master branch.

Discussion
----------

Use the Route annotation from Symfony core

Related to symfony/symfony#25361.

Commits
-------

f35cddb Use the Route annotation from Symfony core
mwhorse46 added a commit to mwhorse46/sym_proj that referenced this issue Apr 16, 2023
This PR was merged into the master branch.

Discussion
----------

Use the Route annotation from Symfony core

Related to symfony/symfony#25361.

Commits
-------

f35cddb Use the Route annotation from Symfony core
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Keep open RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Projects
None yet
Development

No branches or pull requests