Problem/Motivation

We should add autowiring for services in the file module. This removes boilerplate and allows more flexibility when creating new definitions.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#4 3534589-nr-bot_igx6sudu.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3534589

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:

About issue forks

Comments

kim.pepper created an issue. See original summary.

kim.pepper opened merge request !12655

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Active » Needs review
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3534589-nr-bot_igx6sudu.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Needs work » Needs review
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Usually we don't scope things by module, I think this is a duplicate of #3397041: Autowire core modules that do not require explicit configuration

smustgrave made their first commit to this issue’s fork.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

For the feedback on the MR.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Needs work » Needs review

Responded to suggestions. I don't think we should be adding new interfaces in this issue just for the sake of autowiring.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Left some small comments on the MR.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Needs work » Needs review

Addressed feedback.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented

Rebased on 11.x and made use of the FileUsageInterface service alias.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Believe feedback has been addressed here

  • longwave committed 1462e3d2 on 11.3.x 
    task: #3534589 Autowire services in file module

By: kim.pepper
By:...

  • longwave committed 5cc00cc4 on 11.x 
    task: #3534589 Autowire services in file module

By: kim.pepper
By:...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 11.x-dev » 11.3.x-dev
Status: Reviewed & tested by the community » Fixed

On second thoughts maybe per-module (or some other scoping) is easier to review here, trying to do them all in one go is probably too much for a single MR.

Committed and pushed 5cc00cc4bfa to 11.x and 1462e3d2e17 to 11.3.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

longwave closed merge request !12655

smustgrave’s picture

smustgrave commented

Maybe a few modules can be grouped together. But all at once seems very hard to review

Status: Fixed » Closed (fixed)

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