The SandwichPluginManager.php file contains a code comment with the following sentence.

For our Sandwich plugin type, we've specified the @cache.default service be used in the plugin_type_example.services.yml file.

However, the plugin_type_example.services.yml file does not contain any line setting the cache.default service as argument, contrary to what that code comment makes think.

That sentence should be removed. The full comment should be the following one.

    // This sets the caching method for our plugin definitions. Plugin
    // definitions are discovered by examining the $subdir defined above, for
    // any classes with an $plugin_definition_annotation_name. The annotations
    // are read, and then the resulting data is cached using the provided cache
    // backend. The second argument is a cache key prefix. Out of the box Drupal
    // with the default cache backend setup will store our plugin definition in
    // the cache_default table using the sandwich_info key. All that is
    // implementation details, however; all we care about it is that caching for
    // our plugin definition is taken care of by this call.

Punctuation needs to be changed to avoid a comma-splice sentence. Furthermore, all we care about it that caching for our plugin definition is taken care of by this call must be changed to all we care about it is that caching for our plugin definition is taken care of by this call.

I would fix that in this issue, instead of opening a new issue.

Issue fork examples-3109867

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

alberto56 created an issue. See original summary.

valthebald’s picture

@alberto56 this looks like an oversight, thank you for noticing!

avpaderno’s picture

I guess that sentence can be removed. Plugin managers get the cache backend from the parent service (usually, default_plugin_manager).

avpaderno’s picture

Version: 8.x-1.x-dev » 4.0.x-dev
avpaderno’s picture

Version: 4.0.x-dev » 8.x-1.x-dev

 

apaderno changed the visibility of the branch 3109867-remove-comment-about-cache-backend to hidden.

avpaderno’s picture

Status: Active » Needs review

avpaderno’s picture

Version: 8.x-1.x-dev » 4.0.x-dev
andypost’s picture

Looks lie we need to fix tests first

avpaderno’s picture

This issue just change a comment in code; it cannot cause those tests to fails.
Either there is something that needs to be fixed in the other modules, or their tests need to be updated. It is not something that should be fixed in this issue, though.

avpaderno’s picture

Title: SandwichPluginManager comment says @cache.default service used in plugin_type_example.services.yml, but that file does not mention anything about cache » Remove the "For our Sandwich plugin type, we've specified the @cache.default service be used in the plugin_type_example.services.yml file." sentence from a code comment
Category: Support request » Task
Issue summary: View changes
avpaderno’s picture

Actually, 13 seems the number of failing tests when a MR's changes do not cause more tests to fail. (I agree: It would be better if none of the tests fails, but the single issues cannot fix all the failing tests, and I would not postpone them to when all the tests are fixed.)

apaderno changed the visibility of the branch 3109867-8.x-remove-comment-about-cache-backend to hidden.

  • apaderno committed 310e9c86 on 4.0.x
    Issue #3109867: Remove the comment about the cache.default service being...
avpaderno’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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