-
-
Notifications
You must be signed in to change notification settings - Fork 413
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
replace Bundle with the new AbstractBundle class #1514
Conversation
b5544df
to
9e2c829
Compare
It's still the correct place for them in my opinion.
It should be
I usually put this config file at the root of the project, but it'd be okay under
They look like configuration files to me, as such they could be moved to
Kind of templates! |
10d1ba8
to
9cde4be
Compare
Move the bundle from the old "Bundle" to the new "AbstractBundle" system following the best practices outlines in https://symfony.com/doc/current/bundles/best_practices.html
Nothing to see here yet...
see - symfony/symfony-docs#19793
I'm not sure if we should do this yet... Maker has a "leg up" on the other
symfony/*
bundles because we're a bit bleeding edge. Our minimumsymfony/*
is6.4
- but by merging this PR, we would be breaking away from say,FrameworkBundle
that still uses the legacyBundle
class...On the flip side, if we do press forward with this PR - we could reference this change somehow in the docs to show others how we "converted" to the new simplification strategy.
Pinging @nicolas-grekas @javiereguiluz & @yceruto This isn't review ready yet, but feedback on if we should press forward with this would be awesome.
Merge blockers:
src/DependencyInjection/CompilerPass/*
figure out if these should live here still - or if there is a better way...the bundled php-cs-fixer used by the template linter was replaced with a shimsrc/Resources/bin
- This is our "bundled" php-cs-fixer, it needs a new home (not intended to be directly called by the user...)src/Resources/config/php-cs-fixer.config.php
- used by our internal php-cs-fixer when runningmake:*
- make sure this path isnt hard coded.. If it is, fix it to use the newconfig/php-cs-fixer.config.php
path.src/Resources/doc
->docs/
This will done in a separate PR as it will create a messy diff...src/Resources/help
- figure out a home for these files - theAbstractMaker
? (or another parent class) needs to be updated to point the command to the new help locationThis will done in a separate PR as it will create a messy diff...src/Resources/Skeleton
- our "templates" need a new home. These feel like they should live insrc
maybe something likesrc/Skeleton
? hmm....