As far as I can see app.root.factory does not need to be public.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Prioritized changes The main goal of this issue is performance and sense. There will be a minor performance boost from not having this as a public service and it just does not make sense to have this as a public service.
Disruption None
CommentFileSizeAuthor
#5 d8.app-root-private.5.patch400 bytesalexpott
#3 d8.app-root-private.3.patch780 bytesalexpott
d8.app-root-private.patch400 bytesalexpott

Comments

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes
dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Needs work

There is indeed no reason for it, not to be private. You should never access it, but instead 

+++ b/core/core.services.yml
@@ -471,6 +471,7 @@ services:
+    private: true

this is not the way how you define private services http://symfony.com/doc/current/components/dependency_injection/advanced....

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Make app.root.factory private » Make a couple of services not public
Status: Needs work » Needs review
StatusFileSize
new d8.app-root-private.3.patch780 bytes

Okay - let's fix where I c&p'd this from

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Needs work 
+++ b/core/core.services.yml
@@ -488,7 +489,7 @@ services:
   http_negotiation.format_negotiator:
     class: Drupal\Core\ContentNegotiation
-    private: true
+    public: false

This will break the following code: $format = \Drupal::service('http_negotiation.format_negotiator')->getContentType($request);

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Make a couple of services not public » Make app.root.factory private
Status: Needs work » Needs review
StatusFileSize
new d8.app-root-private.5.patch400 bytes

okay let's leave that for another issue.

Last patch :)

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Quickfix

You should not be able to complain about a one line patch.

The last submitted patch, 3: d8.app-root-private.3.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Can't really argue with a one-line performance fix. :)

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 03931c0 on 8.0.x 
    Issue #2457695 by alexpott: Make app.root.factory private

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.