Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mrsinguyen’s picture

Status: Active » Needs review
FileSize
987 bytes
parthipanramesh’s picture

Issue summary: View changes
Status: Needs review » Needs work

patch failed to apply..

xjm’s picture

Title: Remove Unused local variable $translation_es from /core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.php » Remove unused local variables from the translation module
Component: other » translation.module

Let's also check for other unused local variables in the translation module.

xjm’s picture

Priority: Normal » Minor
Rajesh Ashok’s picture

Assigned: Unassigned » Rajesh Ashok
Rajesh Ashok’s picture

Removed unused local variables in the content_translation module.

Rajesh Ashok’s picture

Assigned: Rajesh Ashok » Unassigned
izus’s picture

Status: Needs review » Needs work

there are some unused use statements.
will upload a patch within minutes

izus’s picture

deleted some use statements that seems to be unused.
hopefully this goes green !

izus’s picture

+++ b/core/modules/content_translation/lib/Drupal/content_translation/Form/ContentTranslationDeleteForm.php
@@ -40,7 +40,6 @@ public function getFormId() {
-    $uri = $this->entity->uri('drupal:content-translation-overview');

This one wasn't realy meant, i'll fix it back

izus’s picture

Status: Needs review » Needs work
izus’s picture

here it is again

id.medion’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

The patch cannot be applied.

InternetDevels’s picture

Issue tags: -Needs reroll

Removed tag "Needs Reroll"

eugenesia’s picture

Status: Needs review » Needs work
eugenesia’s picture

Issue tags: +Needs reroll

Patch in #14 couldn't be applied as content_translation_menu() was removed in the latest dev version of the module.

eugenesia’s picture

Rerolled patch #14. Also verified with PhpStorm that the content_translation module doesn't have any unused variables or imported namespaces.

connorwk’s picture

Here at the Austin sprint.
Going to re-roll and review this.

connorwk’s picture

FileSize
813 bytes

Ok so I made a new patch because I realized in the current patch that I was working on re-rolling there are a lot of use statements removed that are not used. This is good work but is out of the scope of this issue and should have a new issue made for it or be put in one already existing for it.
There was only one unused local variable in the content_translation module reported by phpStorm so I made a patch for that.
Should just need review and this issue should hopefully be RTBC.

sun’s picture

Status: Needs review » Reviewed & tested by the community

Agreed that the use statements are out of scope.

  • Commit aedaf11 on 8.x by alexpott:
    Issue #2080559 by izus, connork, eugenesia, InternetDevels, Rajesh Ashok...
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed aedaf11 and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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