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

wip - use the new bundle structure #1514

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jrushlow
Copy link
Collaborator

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 minimum symfony/* is 6.4 - but by merging this PR, we would be breaking away from say, FrameworkBundle that still uses the legacy Bundle 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...
  • src/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 running make:* - make sure this path isnt hard coded.. If it is, fix it to use the new config/php-cs-fixer.config.php path.
  • src/Resources/doc -> doc/
  • src/Resources/help - figure out a home for these files - the AbstractMaker? (or another parent class) needs to be updated to point the command to the new help location
  • src/Resources/Skeleton - our "templates" need a new home. These feel like they should live in src maybe something like src/Skeleton? hmm....
  • Test, test in ci, test in linked apps, test some more, run some tests....

@jrushlow jrushlow added the Minor Minor Enhancement label Apr 17, 2024
@jrushlow jrushlow added the Status: Needs Work Additional work is needed label Apr 17, 2024
@yceruto
Copy link
Member

yceruto commented Apr 17, 2024

src/DependencyInjection/CompilerPass/* figure out if these should live here still - or if there is a better way...

It's still the correct place for them in my opinion.

src/Resources/bin - This is our "bundled" php-cs-fixer, it needs a new home (not intended to be directly called by the user...)

It should be <root>/bin/ instead (according best practices document)

src/Resources/config/php-cs-fixer.config.php - used by our internal php-cs-fixer when running make:* - make sure this path isnt hard coded.. If it is, fix it to use the new config/php-cs-fixer.config.php path.

I usually put this config file at the root of the project, but it'd be okay under <root>/config/ as well

src/Resources/doc -> doc/

<root>/docs/ in plural (according to best practices doc)

src/Resources/help - figure out a home for these files - the AbstractMaker? (or another parent class) needs to be updated to point the command to the new help location

They look like configuration files to me, as such they could be moved to <root>/config/help/ dir

src/Resources/Skeleton - our "templates" need a new home. These feel like they should live in src maybe something like src/Skeleton? hmm....

Kind of templates! <root>/skeleton/ lower case 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Minor Minor Enhancement Status: Needs Work Additional work is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants