Problem/Motivation

This is a recommendation from best practice to use DI instead of Drupal:: directly for Request

\Drupal calls should be avoided in classes, use dependency injection instead

Steps to reproduce

Proposed resolution

Create the constructor and receive this class and apply dependency injection to this

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork current_page_crumb-3333996

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

RenatoG created an issue. See original summary.

RenatoG opened merge request !5

renatog’s picture

renatog commented
Status: Active » Needs review

Done! MR created and is ready to be merged: https://git.drupalcode.org/project/current_page_crumb/-/merge_requests/5

greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented
Status: Needs review » Needs work

This needs a rebase because I merged 4.

renatog’s picture

renatog commented
Status: Needs work » Needs review

MR rebased.

Now is mergeable: https://git.drupalcode.org/project/current_page_crumb/-/merge_requests/5...

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

prudloff’s picture

prudloff commented
Status: Needs review » Needs work

phpcs reports a problem: https://git.drupalcode.org/project/current_page_crumb/-/jobs/4559870

liam morland made their first commit to this issue’s fork.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Needs work » Needs review
liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

All checks are now passing.

prudloff’s picture

prudloff commented
Status: Needs review » Fixed

The constructor of a service is considered internal (https://www.drupal.org/about/core/policies/core-change-policies/bc-polic...) and could change in the future. So I used composition instead of extending PathBasedBreadcrumbBuilder.

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.

  • prudloff committed 8d2578ba on 8.x-1.x authored by renatog 
    feat: #3333996 Use dependency Injection instead of Drupal call directly...

Status: Fixed » Closed (fixed)

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