Skip to content
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

Tried to clarify private services #4656

Merged
merged 1 commit into from
Jan 3, 2015
Merged

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented Dec 15, 2014

Q A
Doc fix? yes
New docs? kind of
Applies to all
Fixed tickets #4524

It was a though one to describe, as I wanted to make it clear and short. I'm really happy if people can review this and provide other, better, alternatives!

this will result in an inlined instantiation (e.g. ``new PrivateFooBar()``)
inside this other service, making it publicly unavailable at runtime.
What makes private services special, is that they are converted from services
to inlined instantiation (e.g. ``new PrivateThing()``) when they are only
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be OK to mention that Symfony does this for performance reasons?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we have to mention that it is JUST for performance reason.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added it

@wouterj
Copy link
Member Author

wouterj commented Dec 28, 2014

I applied the comments, thanks!

@timglabisch
Copy link
Contributor

awesome 👍

@weaverryan weaverryan merged commit d89ad21 into symfony:2.3 Jan 3, 2015
weaverryan added a commit that referenced this pull request Jan 3, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

Tried to clarify private services

| Q   | A
| --- | ---
| Doc fix? | yes
| New docs? | kind of
| Applies to | all
| Fixed tickets | #4524

It was a though one to describe, as I wanted to make it clear and short. I'm really happy if people can review this and provide other, better, alternatives!

Commits
-------

d89ad21 Tried to clarify private services
weaverryan added a commit that referenced this pull request Jan 3, 2015
@weaverryan
Copy link
Member

Hi guys!

I merged this in, but then realized I wanted to make some changes above these changes and reorganize a few things. Please check out #4748.

Thanks!

weaverryan added a commit that referenced this pull request Jan 3, 2015
weaverryan added a commit that referenced this pull request Jan 3, 2015
weaverryan added a commit that referenced this pull request Jan 4, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

Re-reading private service section

| Q   | A
| --- | ---
| Doc fix? | yes
| New docs? | kind of
| Applies to | all
| Fixed tickets | n/a

Hi guys!

This follows #4656. I merged that, but then realized that if you read the wider section, the top was still talking about private services as if their benefit was to *not* allow fetching directly (whereas the true emphasis now is on performance, and how `get()` may or may not work.

Thanks!

Commits
-------

8f5e210 Re-wording based on Wouter's recommendation
0f86a86 [#4656] Re-reading private service section
weaverryan added a commit that referenced this pull request Jan 4, 2015
* 2.3:
  Re-wording based on Wouter's recommendation
  update text to use SetHandler (not ProxyPassMatch)
  cache_csrf_form
  [Book][Security] Add isPasswordValid doc as in 2.6
  [#4656] Re-reading private service section
weaverryan added a commit that referenced this pull request Jan 4, 2015
* 2.5:
  Re-wording based on Wouter's recommendation
  update text to use SetHandler (not ProxyPassMatch)
  cache_csrf_form
  [Book][Security] Add isPasswordValid doc as in 2.6
  [#4656] Re-reading private service section
weaverryan added a commit that referenced this pull request Jan 4, 2015
* 2.7:
  Re-wording based on Wouter's recommendation
  update text to use SetHandler (not ProxyPassMatch)
  cache_csrf_form
  [Book][Security] Add isPasswordValid doc as in 2.6
  [#4656] Re-reading private service section
  Update code example to fit description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants