There are many advantages of using ControllerBase.

This issue removes the hidden dependency on global function l() and t() and improves testability.

CommentFileSizeAuthor
#2 interdiff-0-2.txt1.03 KBmartin107
#2 use-ControllerBase-2.patch3.22 KBmartin107
use-ControllerBase.patch3.37 KBrpayanm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, use-ControllerBase.patch, failed testing.

martin107’s picture

martin107 CreditAttribution: martin107 commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
use-ControllerBase-2.patch3.22 KB
interdiff-0-2.txt1.03 KB

Locally I can reproduce the SimpleTestExampleTest failure on head so it is not this issue that is causing the break.

So I have fixed two nit picks before moving on to find/create a broken HEAD issue.

new Url('page_example_simple');

1) Looking at the Url constructor documentation it say it is only use this in extreme cases Url::fromRoute() is preferred because it is self documenting.
2) Converted one more t() into $this->t().

Updated issue summary to mention dependency injection.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented

Totally the way to go, since the base class and injected t() is the best-practice for this kind of thing.

Patch still applies... Let's see about the testbot.

Mile23 queued 2: use-ControllerBase-2.patch for re-testing.

  • Mile23 committed 919d085 on 8.x-1.x authored by martin107 
    Issue #2469093 by martin107, rpayanm: Extends the ControllerBase and use...
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Status: Needs review » Fixed

Committed and pushed. Thanks!

Status: Fixed » Closed (fixed)

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