Use symfony 3.3 features

[GuilhemN]

With Flex coming and the refactoring of the docs, I think we should push sf 3.3 di features and also begin to accustom people to sf 4.0 ideas.

ping @weaverryan

[weaverryan]

Awesome! This was on my list!

We need this for the docs: we need to be able to assume that new users (at least) have a certain set of config.

This config is subjective, let's try to find a really "good" first attempt :). The only part I'm not sure about is public: false - I feel like we should spend some time training users to not use the container as a locator, then begins taking this ability away a bit later.

Oh, maybe add a few comments. Like for the main service loader, comment that it adds a service for each class and that you can add whatever directories you need.

Thanks!

[yceruto]

I was wondering if we could use %kernel.project_dir% instead of ../.. here and elsewhere? DX and verbose.

[weaverryan]

It's logical, but it's longer and I kind of think it's harder. The ../ is very clear :)

[yceruto]

Well, I thought that %kernel.project_dir% was quite clear :), because:

• DX: I don't have to think where is the file.yml placed to write/read relatived paths (thinking in new configurations),
• DX: Also, it account with IDE auto-completation (may be more hard for those who don't have it),
• DX: Beside, it would allows us move these files without to break its internal configuration (i.e relative paths are fragile into flexible structures),
• config.yml use %kernel.root_dir% instead of relative paths (a big example):

framework:
    router:
        resource: '%kernel.root_dir%/config/routing.yml' # just 'routing.yml' work!

But I could be wrong and it's not enough of course :)

[GuilhemN]

I prefer keeping the relative path too as everyone knows this syntax so a newcomer can understand it while %kernel.project_dir% is specific to symfony.

[GuilhemN]

I added some comments.

The only part I'm not sure about is public: false - I feel like we should spend some time training users to not use the container as a locator, then begins taking this ability away a bit later.

As autowiring is also enabled by default, I think we can make this move.
Wdyt @fabpot as you changed this in the first place?

[xabbuh]

In the documentation, we place custom voters in a Security subnamespace instead of using Voter (see http://symfony.com/doc/current/security/voters.html).

[GuilhemN]

Thanks for the catch, updated.

See also symfony/recipes#42 for the related PR for Flex.

[weaverryan]

Other than the open question about public: true under _defaults, 👍

Awesome job @GuilhemN with the comments - these are SO clear. Seriously - <3 them.

[GuilhemN]

You and @fabpot did a big part of the job so a big thank to you too ☺

I hope this will be merged soon it makes symfony much more accessible and is a great step forward imo.

About public, it's not the most important part of the PR so I removed it for now, we can discuss it later ☺

[fabpot]

I would indeed keep public: false. Or we keep it as true here in Symfony SE and switch it to true for Flex. WDTY?

[nicolas-grekas]

public: false is a good practice, it allows auto-removal of unused service loaded by PSR-4 discovery, and inlining in the container + encourages using regular injection over injecting the container to use it as an opaque service locator. I'll all 👍 for making public: false the default with 3.3.

[GuilhemN]

When looking at the docs, using the container as a service locator is still a big thing, so i'm totally for public: false but @weaverryan has a point saying people are not ready for private services yet.
So objectively, I think we should delay this change until we revamp the docs to push injecting dependencies.

[weaverryan]

I'm also now leaning towards public: false here in the SE. I think this will cause some issues for users, but it's also easy to fix (i.e. add public: true to your service or [better] use DI / some of the new injection techniques). And since we're moving towards a type-based system (i.e. where you ask the container for a service by class/interface, not by id), this will push that forward.

Of course, we should also add a comment above this new line - something like:

# this means you cannot fetch services directly from the container via $container->get()
# if you need to do this, you can override this setting on individual services
public: false

[weaverryan]

@GuilhemN this would definitely force us to do that docs-revamping right now :) - but we can do that

[GuilhemN]

@weaverryan ok I'm adding it back with your comments :)

[fabpot]

Thank you @GuilhemN.