Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
Update some examples using 3.3 features#7877
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
0839806 to5fc9ffeComparebest_practices/templates.rst Outdated
| # add Service/ to the list of directories to load services from | ||
| AppBundle\: | ||
| resource:'../../src/AppBundle/{Service,Updates,Command,Form,EventSubscriber,Twig,Security}' |
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.
theUpdates namespace looks wrong to me
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.
Indeed, good catch!
form/create_custom_field_type.rst Outdated
| AppBundle\Form\Type\GenderType: | ||
| arguments: | ||
| -'%genders%' | ||
| tags:[form.type] |
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 about usingautoconfigure instead?
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.
same in other places
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.
In fact, imo we should assert that the user has the default SE config and remove config blocks when they're useless. We could just add a note to explain how to declare services manually once for each feature.
Wdyt?
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 good
GuilhemN commentedMay 12, 2017
I think I'll split this PR in smaller PRs like#7902 because it's much longer to respect#7877 (comment). |
weaverryan commentedMay 16, 2017
@GuilhemN In fact, I apologize, because I'm completely running over your work :/. There are just so many entries to update, that I've been keeping a list locally and running through them (and making GIANT PRs). That's not idea, but I don't think we'll be able to update everything in time without doing that. However, thatdoes mean that those PR's need review (and also, possible some of the most important docs could/should be proofread afterwards ). However, you do have a few entires I don't. Could you rebase? So far, I'm skipping the following directories and saving for later:
Thanks! |
This PR was merged into the master branch.Discussion----------Update best_practices/templates.rstFirst example of a doc asserting we're using the 3.3 SE (#7877 (comment)).Commits-------8769a30 Update best_practices/templates.rst
GuilhemN commentedMay 16, 2017
No problem, this PR didn't take me long to do :)
done ;) Do you still need help for the remaining parts? |
form/type_guesser.rst Outdated
| <services> | ||
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:schemaLocation="http://symfony.com/schema/dic/services | ||
| http://symfony.com/schema/dic/services/services-1.0.xsd"> |
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.
this change should probably also be made on older branches
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.
done in#7915, I also found many others...
reference/dic_tags.rst Outdated
| app.custom_routing_loader: | ||
| class:AppBundle\Routing\CustomLoader | ||
| AppBundle\Routing\CustomLoader: | ||
| tags:[routing.loader] |
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.
could be merged with the previous line
reference/dic_tags.rst Outdated
| my_service: | ||
| class:Monolog\Processor\IntrospectionProcessor | ||
| Monolog\Processor\IntrospectionProcessor: | ||
| tags:[monolog.processor] |
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.
could be merged with the previous line
reference/dic_tags.rst Outdated
| app.custom_subscriber: | ||
| class:AppBundle\EventListener\CustomSubscriber | ||
| AppBundle\EventListener\CustomSubscriber: | ||
| tags:[kernel.event_subscriber] |
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.
could be merged with the previous line
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.
indeed, done
de69532 to14b4689Comparexabbuh commentedMay 19, 2017
@GuilhemN Can you resolve the conflicts? :) |
GuilhemN commentedMay 19, 2017
@xabbuh done |
weaverryan commentedMay 21, 2017
Thanks@GuilhemN! |
This PR was merged into the 3.3 branch.Discussion----------Update some examples using 3.3 featuresI updated some examples using 3.3 di features, but there is still a lot to do.Commits-------04903f7 Update some examples using 3.3 features
* 3.4: Encore docs Fixed typo Revert a wrong change from#7877 Remove note about --force option on server:start Small grammatical improvement [DependencyInjection] fix some minor typos remove remaining LdapClient usages add missing XML and PHP config examples explain how to properly override choices Fix custom router loader service declaration
I updated some examples using 3.3 di features, but there is still a lot to do.