Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
Clarify tip for creating a new AppBundle#4702
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
javiereguiluz commentedDec 29, 2014
@xelaris thanks for reporting this issue. You are right about the error generated by |
xelaris commentedDec 29, 2014
@javiereguiluz That sounds good. But there is also the other point. I think in the context of the chapter the reader never needs to create the AppBundle with this command, since it comes out of the box for all maintained versions, also before 2.6. At least the statement is not totally right. Maybe it should be changed to something like:
What do you think? |
javiereguiluz commentedDec 29, 2014
@xelaris you are right. But that note is intended for people using any Symfony version which doesn't include the AppBundle, which are most of past versions. Some people may want to refactor their application to use the new set of best practices, and the first step would be to create the AppBundle. |
xelaris commentedDec 29, 2014
@javiereguiluz I understand the need for this tip, when people want refactor their application. I have changed my commit. IMO this would make it more clear that you can have a version before 2.6 and do not need to (and can not) execute the command, since the AppBundle could be already there. |
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.
What do you think of "If your Symfony installation doesn't come with a pregeneratedAppBundle, you can generate it by hand executing this command:"?
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.
Sounds better, going to change...
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.
I think the word "pregenerated" doesn't exist. Can we use "pre-generated" or "previously generated" instead?
javiereguiluz commentedDec 29, 2014
@xelaris I like your suggestion, but if you read the new phrase carefully: The only thing I don't like is that it exposes a "problem" without explaining the reason. Why doesn't my app include an AppBundle? Have I done something wrong? Have I downloaded the wrong Symfony version? Do I have to update my Symfony version before refactoring its code? |
xelaris commentedDec 29, 2014
What do you think about where the wordexisting explains the reason? |
javiereguiluz commentedDec 30, 2014
@xelaris it's better ... but there are some missing reasons: perhaps you use the Symfony Installer to install a specific version (e.g. 2.3.23) that didn't include the AppBundle yet). |
xelaris commentedDec 30, 2014
@javiereguiluz That's not true. A new project created with |
xabbuh commentedDec 30, 2014
A user could only get Symfony without the AppBundle using the installer if they did it before the pull request in the standard edition including the AppBundle was merged. Though this has been done a few weeks ago when the installier still was in an early state, right? So, I don't think that this really is an issue we need to take into account here. |
wouterj commentedDec 30, 2014
I think we don't have to go into detail here. The best practices state that you create it using And btw, this tip has to be as short as possible, as it's only important for a small group of people. |
javiereguiluz commentedDec 30, 2014
xelaris commentedDec 30, 2014
I have changed the commit to the latest proposal (incl. |
wouterj commentedDec 30, 2014
I would prefer it without "existing", as it only adds confusion imo (and it makes it a bit harder to read). |
The previous description was not quite right, since an AppBundle canalso be there in versions before 2.6.
xelaris commentedDec 30, 2014
Here is another change. I also prefer this shorter, generic description. |
xabbuh commentedDec 30, 2014
👍 I like it. |
weaverryan commentedJan 3, 2015
I like it too guys, a lot :). Thanks for a well-thought-out PR@xelaris! |
This PR was merged into the 2.3 branch.Discussion----------Clarify tip for creating a new AppBundle| Q | A| ------------- | ---| Doc fix? | yes| New docs? | no| Applies to | all| Fixed tickets |The tip seems to be confusing, since the AppBundle is also generated with `symfony new blog 2.3.23` (and 2.5.8). In addition the mentioned `generate:bundle` command does not work in SF 2.3 SE, since it stops with a `The namespace must contain a vendor namespace` error.Commits-------7b9d583 Clarify tip for creating a new AppBundle
weaverryan commentedJan 3, 2015
And@javiereguiluz, I just checked, we're (fortunately) not waiting onsensiolabs/SensioGeneratorBundle#312 for the "vendor namespace" |
The tip seems to be confusing, since the AppBundle is also generated with
symfony new blog 2.3.23(and 2.5.8). In addition the mentionedgenerate:bundlecommand does not work in SF 2.3 SE, since it stops with aThe namespace must contain a vendor namespaceerror.