Problem/Motivation

Once #3542540: Deprecate comment libraries and move to history module is done we no longer need the $moduleHandler and $entityTypeManager properties. We could deprecate them in that issue, but I've separated this to make it easier to review.

We can also switch to autowiring the same time. Thought about constructor property promotion too, but with the difference in param name and property name it's probably not worth it.

Steps to reproduce

Proposed resolution

  • Deprecate $moduleHandler and $entityTypeManager properties
  • Use autowiring in the CommentLinkBuilder service

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3544308

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

mstrelan created an issue. See original summary.

mstrelan opened merge request !13137

mstrelan’s picture

mstrelan commented
Title: [PP-1] Deprecate unused properties in CommentLinkBuilder » Deprecate unused properties in CommentLinkBuilder
Status: Postponed » Needs work
mstrelan’s picture

mstrelan commented
Issue summary: View changes
Status: Needs work » Needs review
dcam’s picture

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

I looked over the changes a few times. I don't have any feedback. They look good to me. Tests are passing. I wondered if it would need deprecation tests, but another recent issue that changed a constructor's parameters didn't require them. So I assume we're good there. Finally, there are no other instances of CommentLinkBuilder::__construct() being invoked. I'm going to RTBC it.

I hope you don't mind that I filled out the change record description with before and after examples. They shouldn't be necessary because people should be depending on the service, but I guess it's better to be safe than sorry.

smustgrave’s picture

smustgrave commented
Issue tags: +Needs Review Queue Initiative

Not much to review here! seems like an excellent deprecation and see nothing wrong.

Edit tab was out of date and @dcam beat me to it!

catch closed merge request !13137

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, thanks!

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

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

Maintainers, please credit people who helped resolve this issue.

  • catch committed f5721872 on 11.x 
    Issue #3544308 by mstrelan, dcam: Deprecate unused properties in...

Status: Fixed » Closed (fixed)

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