Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DI][FrameworkBundle] Hide service ids that start with a dot#26921
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
df5b6c3 toc689088Comparenicolas-grekas commentedApr 20, 2018
ping @symfony/deciders this would be great in 4.1, the current state is clumsy. |
lyrixx commentedApr 20, 2018
I like the idea. But Why did you choose a dot? I think it would be better to choose an underscore even if the meaning of a name starting with a dot in the unix FS is to hide it. The underscore is already used in the framework (or other lib) to mean "it's internal / private". I'm thinking to |
nicolas-grekas commentedApr 20, 2018
Good question :) |
sroze commentedApr 20, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I really like the idea as well. And tbh, hiding things that started with The result of the |
nicolas-grekas commentedApr 20, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
If we were to answer yes to this question, we couldn't do any improvement to any command we publish. The answer must be no. |
lyrixx commentedApr 20, 2018
okay, go for it ;) |
| $definition =$this->resolveServiceDefinition($builder,$serviceId); | ||
| if (!$definitioninstanceof Definition || !$showPrivate && !$definition->isPublic()) { | ||
| if ($showHiddenxor'.' ===$serviceId[0] ??null) { |
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.
Can we use parentheses here? At least I have no idea by reading the code whetherxor takes precedence over?? or vice versa.
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.
Is this actually what we want at all? Won't we skip non hidden service now if$showHidden istrue?
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.
parentheses added
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.
Can we do that in the other files too that use the same condition? :)
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.
never mind, I was looking at an outdated diff
c689088 tof809d72Comparenicolas-grekas commentedApr 20, 2018
That's correct: by default, you get non-hidden services, and if you add |
| using the <info>--show-private</info> flag: | ||
| <info>php %command.full_name% --show-private</info> | ||
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.
You should add some docs about the new flag.
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.
added
f809d72 tocea051eComparefabpot commentedApr 20, 2018
Thank you@nicolas-grekas. |
… a dot (nicolas-grekas)This PR was merged into the 4.1-dev branch.Discussion----------[DI][FrameworkBundle] Hide service ids that start with a dot| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | yes| Tests pass? | yes| Fixed tickets |#26630| License | MIT| Doc PR | -Now that services are private by default, `debug:container` should display them by default.In fact, the public/private concept should not trigger a difference in visibility for this command.Instead, I propose to handle service ids that start with a dot as hidden services.PR should be ready.(For reference, I tried using a tag instead in#26891, but that doesn't work in practice when working on the implementation.)Commits-------cea051e [DI][FrameworkBundle] Hide service ids that start with a dot
…as-grekas)This PR was merged into the 4.1-dev branch.Discussion----------[FrameworkBundle] Hide some lock-related services| Q | A| ------------- | ---| Branch? | 4.1| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -I don't know why, but I missed these in#26921Commits-------d06d8b2 [FrameworkBundle] Hide some lock-related services
Uh oh!
There was an error while loading.Please reload this page.
Now that services are private by default,
debug:containershould display them by default.In fact, the public/private concept should not trigger a difference in visibility for this command.
Instead, I propose to handle service ids that start with a dot as hidden services.
PR should be ready.
(For reference, I tried using a tag instead in#26891, but that doesn't work in practice when working on the implementation.)