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] Add make:bundle command #87

Open
maidmaid opened this Issue Dec 4, 2017 · 17 comments

Comments

Projects
None yet
8 participants
@maidmaid

maidmaid commented Dec 4, 2017

To create a reusable bundle is a little boring. SensioGeneratorBundle have a command for this. Why not have a similar command here?

@flip111

This comment has been minimized.

flip111 commented Dec 4, 2017

There are quite some templates for bundle that need to be checked if it's still best practice for symfony 4 https://github.com/sensiolabs/SensioGeneratorBundle/tree/master/Resources/skeleton/bundle

@maidmaid

This comment has been minimized.

maidmaid commented Dec 10, 2017

As a reminder, here the files generated by SensioGeneratorBundle:

src/Acme/BlogBundle
├── AcmeBlogBundle.php
├── Controller
│   └── DefaultController.php
├── DependencyInjection
│   ├── AcmeBlogExtension.php
│   └── Configuration.php
├── Resources
│   ├── config
│   │   └── services.yml
│   └── views
│       └── Default
│           └── index.html.twig
└── Tests
    └── Controller
        └── DefaultControllerTest.php
@maidmaid

This comment has been minimized.

maidmaid commented Dec 10, 2017

We should provide the minimum. So, DefaultController.php and index.html.twig are already handled by a maker, I think we can remove them. Same thing with DefaultControllerTest.php. According to Best Practices for Reusable Bundles and essential files, the next structure should look well:

.
├─ .gitignore
├─ composer.json
├─ phpunit.xml.dist
├─ AcmeBlogBundle.php
├─ README.md
├─ LICENSE
├── DependencyInjection
│   ├── AcmeBlogExtension.php
│   └── Configuration.php
└─ Resources
    ├─ config
    │   └── services.yml
    └─ doc
       └── index.md
@flip111

This comment has been minimized.

flip111 commented Dec 10, 2017

Drop some additional files that were also not generated by sensio. I think it's a bit too much to preselect a version control system and a package manager and a testing framework. Let's keep it to just the symfony files.

<your-bundle>/
├─ AcmeBlogBundle.php
├─ README.md
├─ LICENSE
├─ DependencyInjection/
│   ├─ AcmeBlogExtension.php
│   └─ Configuration.php
└─ Resources/
    ├─ config/
    │  └─ services.yaml
    └─ doc/
       └─ index.rst
@maidmaid

This comment has been minimized.

maidmaid commented Dec 10, 2017

it's a bit too much to preselect a version control system

We're talking about reusable bundle; Git will be used in 99% of cases.

and a package manager

composer.json file is essential, at least for autoloading.

and a testing framework.

It's maybe more relevant to generate phpunit.xml.dist when we make unit/functional test for the very first time.

@flip111

This comment has been minimized.

flip111 commented Dec 10, 2017

We're talking about reusable bundle; Git will be used in 99% of cases.

It's just too far of a stretch to assume that it will be a reusable bundle in it's own repository. People can just start the bundle in their code base and break it out later, at which point adding a .gitignore is a small effort which only occurs seldomly.

composer.json file is essential, at least for autoloading.

There is already a composer.json in the root of the symfony project if you are using the default symfony installation method.

@flip111

This comment has been minimized.

flip111 commented Dec 10, 2017

I started working on a PR. But i ran into some architect difficulties of which i don't know why certain choices were made. The MakerCommand delegates interact to the Bundle classes

https://github.com/symfony/maker-bundle/blob/master/src/Command/MakerCommand.php#L86

but it doesn't pass $output which is used in the interact method of the sensio bundle https://github.com/sensiolabs/SensioGeneratorBundle/blob/master/Command/GenerateBundleCommand.php#L116

If i add this parameter i also have to change all the other Bundle classes even if they don't use output ...

another thing is that i need to add a getQuestionHelper method to the MakerCommand because i need to instantiate a custom QuestionHelper https://github.com/sensiolabs/SensioGeneratorBundle/blob/master/Command/Helper/QuestionHelper.php

Please advice on the $output parameter issue.

@7thcubic

This comment has been minimized.

7thcubic commented Jan 4, 2018

Command cannot create the bundle in src folder due to the App namespace taking up the src folder. so if this command is ever implemented as a way for developers to create third party bundles on symfony 4. the bundles have to be created at the root folder

@alexiswbr

This comment has been minimized.

alexiswbr commented Jan 4, 2018

I created a "bundles" folder at the root, and in composer.json I added this line "": "bundles/":

"autoload": {
    "psr-4": {
        "App\\": "src/",
        "": "bundles/"
    }
},

This works well, it could be a good solution

@flip111

This comment has been minimized.

flip111 commented Jan 4, 2018

@7thcubic bundle as sub-namespace of App namespace is not going to work ?

@7thcubic

This comment has been minimized.

7thcubic commented Jan 5, 2018

Lets say i am developing a shared bundle for Acme company, the namespace would start with Acme, if the bundle was created in src folder in symfony4, based on the PSR-4 it would require the namespace to start with App\Acme, how would that work?

so bundles have to be out of source folder if the prefix of the bundle namespace does not start with App.

There is nothing preventing you from creating App prefixed bundles, its just that when a developer is trying to write a shared bundle you can place it in src like how it was done in sf3.

@codedmonkey

This comment has been minimized.

Contributor

codedmonkey commented Jan 6, 2018

@7thcubic That's a solution but if you perform composer diagnose you can see this is not recommended for performance issues. Aside from that, I am 100% against the idea of a new bundle still generating a PSR-0 style directory structure.

@weaverryan I'm interested to hear a comment on the issue raised by @flip111:

The MakerCommand delegates interact to the Bundle classes but it doesn't pass $output which is used in the interact method of the sensio bundle

How is the interaction going to work? I've been wanting to make this PR as well but how the interaction is going to work is still unclear, and I'm assuming you have an idea of how you would like it to work. Hopefully you could shed some light on this, so more people can try to help with the more complicated commands.

@javiereguiluz

This comment has been minimized.

Member

javiereguiluz commented Jan 24, 2018

Here's an unpopular comment: what if we close this proposal as "won't fix"?

First, we can't generate "app bundles" because that concept is completely deprecated in Symfony.

Second, we could generate "reusable or third-party bundles" ... but generating a reusable bundle inside a Symfony application is "wrong". When you want to publish the bundle in some public or private repo, you should remove all the Symfony app files unrelated to the bundle.

What do you think?

@flip111

This comment has been minimized.

flip111 commented Jan 24, 2018

@javiereguiluz are you saying it's better not to generate a bundle skeleton out of a principled way how the new symfony is setup? Creating a bundle is still in symfony 4.* though. For technical reasons i think it would still be good to automate this part for "Rapid Bundle Development". The part where you have to remove all the symfony app files are not on that page (or i can not find it, maybe it's somewhere??) .. Sounds to me there are more steps involved now, perhaps another opportunity to automate that part as well.

@codedmonkey

This comment has been minimized.

Contributor

codedmonkey commented Jan 24, 2018

I can certainly agree with your reasons since the point of this bundle is to create clean code. A bundle doesn't have to be more than a single class, all other configuration is optional. The old generator created a bulky example skeleton for bundles but the documentation seems to be good enough for anyone who is trying to attempt to make a bundle.

@bocharsky-bw

This comment has been minimized.

bocharsky-bw commented Jun 18, 2018

I think having this command would be cool, at least for reusable third-party bundles. We may bootstrap directory structure with some files and correct namespaces - it's a routine work that we can avoid that will reduce simple typos. And we can require to specify a path to a directory from users where this bundle should be stored, recommending to store it outside of their Symfony project. Users still may generate the bundle inside their Symfony project, but this way they would need to configure autoloading manually in their composer.json

@mahono

This comment has been minimized.

mahono commented Aug 6, 2018

I think people could live without a make:bundle command.

But: The make commands need to be able to put files in any directory. Currently it seems that they can only put stuff into src/ which is a good start.. but bundle developers need a bit more flexibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment