Part of meta-issue #2002650: [meta, no patch] improve maintainability by removing unused local variables

File /core/modules/datetime/datetime.module

Line 816: Unused local variable $has_time
Line 864: Unused local variable $has_time
Line 850: Unused local variable $has_time
Line 857: Unused local variable $has_time
Line 843: Unused local variable $has_time
Line 998: Unused local variable $title
Line 1052: Unused local variable $hour

CommentFileSizeAuthor
#6 2080067-remove_unused_variables-6.patch2.35 KBnetsensei
#6 interdiff.txt1.01 KBnetsensei
#4 2080067-remove_unused_variables-4.patch1.01 KBbeowulf1416
#1 2080067-remove_unused_variable-1.patch1.54 KBbeowulf1416

Comments

beowulf1416’s picture

beowulf1416 commented
Status: Active » Needs review
StatusFileSize
new 2080067-remove_unused_variable-1.patch1.54 KB

removed unused variable $has_time. I was unable to run DateTimePlus test because I haven't installed the Intl PECL extension

yesct’s picture

yesct commented

@beowulf1416 Thanks for making so many patches. How are you feeling about them?

I added some reviewer notes to the meta. If you review a few of these unused var patches, and have advice to add to the meta, please go ahead and edit that meta issue summary. Anyone can edit it.

If you jump into irc http://drupal.org/irc into #drupal-contribute or the core mentoring office hours https://drupal.org/core-mentoring , we can help match you up with some different issues in some other areas.

CaptainWonky’s picture

CaptainWonky commented

Patch applies ok.

All instances of $has_time have been removed, so the patch works as far as the issue title is concerned.

However, the issue summary includes two other variables: $title and $hour

$title seems to be an unused local variable.

$hour is used, but I assume it's the statement $hour = 0 which needs to be removed, since $hour is not used afterwards.

beowulf1416’s picture

beowulf1416 commented
StatusFileSize
new 2080067-remove_unused_variables-4.patch1.01 KB

removing $hour and $title also.
great eyes!

michaelhiiva’s picture

michaelhiiva commented
Status: Needs review » Needs work

The #4 patch is not quite correct, should include changes made in #1 as the patch has not been reviewed and commited. Changes in #4 otherwise seem fine and patch applies. Changing status to needs work.

netsensei’s picture

netsensei commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.01 KB
new 2080067-remove_unused_variables-6.patch2.35 KB

Okay. Rerolled the patch. With an interdiff showing the integration of #4 in #1.

areke’s picture

areke commented
Issue summary: View changes

This applies cleanly and doesn't break any functionality; it works well. Thank you!

areke’s picture

areke commented
Status: Needs review » Reviewed & tested by the community
catch’s picture

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

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

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