Problem/Motivation

This Url::fromRoute() also creates a $route_name variable: https://git.drupalcode.org/project/admin_toolbar/-/blob/28a6dfd1fb1652be...
But this variable is never used.

Steps to reproduce

Proposed resolution

Stop creating the variable and simply call Url::fromRoute() with the route name.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork admin_toolbar-3590488

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

prudloff created an issue. See original summary.

garvitasakhrani’s picture

garvitasakhrani commented

I am working on it.

garvitasakhrani opened merge request !212

garvitasakhrani’s picture

garvitasakhrani commented
Status: Active » Needs review

Removed the unused variable from SearchLinks.php.
Please review the changes. Thankyou.

angel_devoeted’s picture

angel_devoeted commented

Tested and reviewed the changes locally.
The diff is clean, the syntax check passes, and it correctly removes the unused inline $route_name assignment while leaving the rest of the file intact.

Looks good!

dydave’s picture

dydave commented

Thanks a lot everyone!

Great catch!

Indeed ... The MR looks good ...

Just need a tiny bit of time to be able to merge all this.

I'll try coming back to this issue as soon as I can find a bit of time.

Thanks again!

dydave’s picture

dydave commented
Status: Needs review » Fixed

Really sorry for the slow reply on this! 😅
Once again thanks a lot everyone for your help with this issue, it's greatly appreciated! 🙏

Great catch Pierre (@prudloff)! Thanks a lot! 👍
Great clean-up job Garvita (@garvitasakhrani)! 🤩
Thanks Angel (@angel_devoeted) also for the review!

I have reviewed, tested the merge request locally and since all the jobs and tests were still passing 🟢, I went ahead and merged the changes above at #7.

Everyone has been credited on this issue and since I don't see any follow-up to this ticket, marking it as Fixed, for now.

Feel free to let us know if you have any questions on any of the recent code changes or this ticket in general, we would certainly be happy to help.
Thanks again everyone for your interest in the Admin Toolbar module! 😊

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.

  • dydave committed 707d7613 on 3.x authored by xjm 
    Issue #3590488 by xjm, kingdutch: Removed unnecessary call to flush...
dydave’s picture

dydave commented
Related issues: +#3590488: Unused variable in SearchLinks.php

The commit message from #10 is wrong and should belong to related issue:
#3586889: Flushing caches inside an update hook is dangerous and can lead to data integrity problems (as well as poor update performance)

dydave’s picture

dydave commented
Related issues: -#3590488: Unused variable in SearchLinks.php +#3586889: Flushing caches inside an update hook is dangerous and can lead to data integrity problems (as well as poor update performance)

Status: Fixed » Closed (fixed)

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