-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Update book to comply with best practices #4431
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
Comments
Great idea! |
@wouterj I just un-checked the boxes because I added my PR's to this list. This will help me keep track of my work on this as well. Thanks! |
@wouterj 👎 best practices are to be used for non open source projects. If we remove all the "open source best practices", this will impact a lot the quality of open sourced bundles. Best practices is the best and the only place where we should apply them in documentation. |
@mickaelandrieu obviously, we disagree with you here. The documentation should be about people using Symfony for their own projects. Having a small chapter for bundle contributors might be great, but that should not be the main focus of our docs. |
And btw, we already have such chapter: http://symfony.com/doc/current/cookbook/bundles/best_practices.html |
This PR was merged into the 2.3 branch. Discussion ---------- Comply with best practices, Round 2 | Q | A | --- | --- | Doc fix? | yes | New docs? | no | Applies to | all | Fixed tickets | partly #4431 Less other fixes, more find/replace this time to speed up the process. I have one question this time: * Should we just mention the use of the param converter (the current case), or update all simple controller examples to the use param converter? Commits ------- 5ee9791 Some fixes fdc460d Minor standard fix for best practices guide 2a97453 Minor tweak a29f9fb Don't use form() helper 4ef1ef3 Apply best practices to forms de4fcf4 Use AppBundle instead of AcmeStoreBundle c8ce507 Other minor fixes 3c71b6d Use AppBundle instead of AcmeDemoBundle
This PR was merged into the 2.3 branch. Discussion ---------- Use AppBundle whenever it's possible | Q | A | ------------- | --- | Doc fix? | yes | New docs? | no | Applies to | 2.3+ | Fixed tickets | - This PR replaces `AcmeFooBundle`, `AcmeHelloBundle` and `AcmeDemoBundle` by `AppBundle` whenever it's possible in the Cookbook. This is the first step into making the cookbook compliant with the recent good practices promoted by Symfony (the book is being made compliant in #4431). If we change everything to comply with the good practices, the pull request will be massive and it will take months. That's why I think it's better to do it in several steps using different pull requests. By the way, this PR also contains some minor tweaks about using the new `app/config/services.yml` file. I can revert those changes if you want it. Commits ------- c6580f8 Use AppBundle whenever it's possible
…terJ) This PR was merged into the 2.3 branch. Discussion ---------- Update book to comply with best practices, round 3 | Q | A | --- | --- | Doc fix? | yes | New docs? | yes | Applies to | all | Fixed tickets | #4431 Commits ------- 3ab53a6 Fixed some of the comments c6ff013 Made http cache chapter best-practices-compatible and lots of other fixes 4840768 Made propel chapter best-practices-compatible and lots of other fixes a272f99 Made testing chapter best-practices-compatible and lots of other fixes c49f75d Made validation chapter best-practices-compatible
@wouterj I've reviewed the chapter about Security and I've found no changes needed to comply with the new best practices. If you agree, you probably can update the TODO list of this PR to mark Security as "done". Thanks. |
I agree and checked the item. |
I think we're ready here |
Some people wanted to follow the progress, so I've created a task list for at least the book articles. The quick tour will be updated by #4374 and the cookbook will not be as important as these other 2 sections, they'll properbly follow after 2.6 is released.
I've ordered them on priority.
The text was updated successfully, but these errors were encountered: