Make WordPress Core

Opened 16 months ago

Closed 10 months ago

Last modified 6 months ago

#57397 closed defect (bug) (fixed)

Twenty Nineteen: Remove Unused function parameter and variable.

Reported by: upadalavipul's profile upadalavipul Owned by: audrasjb's profile audrasjb
Milestone: 6.3 Priority: normal
Severity: normal Version: 5.0
Component: Bundled Theme Keywords: has-patch commit
Focuses: coding-standards Cc:

Description

I have reviewed twenty-nineteen theme codes and I found some of the function parameters and variables Unused. So I have to remove it from the below files.

Files:

  • twentynineteen/comments.php
  • twentynineteen/inc/template-functions.php

Attachments (2)

57397.patch (1.6 KB) - added by upadalavipul 16 months ago.
57397.2.patch (2.0 KB) - added by viralsampat 6 months ago.
I have worked on another theme and found same issue, Here I have added my patch for another theme.

Download all attachments as: .zip

Change History (21)

#1 @sabernhardt
16 months ago

  • Component changed from General to Bundled Theme

#2 @hellofromTonya
13 months ago

  • Version trunk deleted

Hello @upadalavipul,

Welcome to back to WordPress Core's Trac!

I'm doing prep before tomorrow's 6.2 RC1 and noticed this ticket's Version is set to trunk (saw it in Report 40). The code for this ticket was not introduced in 6.2. So I'll remove trunk as the Version.

#3 @SergeyBiryukov
13 months ago

  • Focuses coding-standards added
  • Milestone changed from Awaiting Review to 6.3

This ticket was mentioned in PR #4225 on WordPress/wordpress-develop by @mukesh27.


13 months ago
#4

  • Keywords has-patch added

#5 follow-up: @costdev
13 months ago

  • Version set to 5.0

#6 @mukesh27
13 months ago

Hi there!

Thanks for the ticket.

In PR #4225 I have used different approaches to solve the issue.

#7 @costdev
13 months ago

  • Keywords commit added

PR 4225 looks good to me and adjusts the add_filter() calls to reduce the number of accepted arguments.

Adding for commit consideration.

#9 @sabernhardt
13 months ago

  1. The twentynineteen_post_thumbnail_sizes_attr filter shows the default priority and accepted arguments , 10, 1. The two filters could be consistent.
  2. The twentynineteen_nav_menu_link_attributes DocBlock has a typo we could fix (either here or on #57840): "Adjustments to menu attributes tot support WCAG 2.0 recommendations for flyout and dropdown menus."
  3. I like that the PR uses the variable for comments navigation, but those translations probably should be replaced anyway (on a new ticket, #58149). Separating "Previous" and "Next" from "Comments" can result in faulty translations.
Last edited 12 months ago by sabernhardt (previous) (diff)

@sabernhardt commented on PR #4225:


12 months ago
#10

Changeset 55655 replaced the comments navigation text strings for better translations, so the $comments_text variable is already removed from comments.php.

#11 @sabernhardt
12 months ago

  • Keywords changes-requested added; commit removed

#12 @sabernhardt
12 months ago

  • Keywords changes-requested removed

#13 @mukesh27
10 months ago

  • Keywords commit added

PR 4225 get several approvals. @SergeyBiryukov Could you please review it.

Adding for commit consideration.

#14 @audrasjb
10 months ago

Self assigning for final review 👀

#15 @audrasjb
10 months ago

  • Owner set to audrasjb
  • Status changed from new to reviewing

#16 @audrasjb
10 months ago

  • Status changed from reviewing to accepted

This looks good. I also spotted a few other docblock issues or improvements in this file and will commit them directly.

#17 @audrasjb
10 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 55963:

Twenty Nineteen: Remove unused function parameters and variables.

This changeset removes unused parameters and variables, fixes a typo, and improves some docblocks as per documentation standards.

Props upadalavipul, mukesh27, costdev, sabernhardt, hellofromtonya, audrasjb.
Fixes #57397.

@viralsampat
6 months ago

I have worked on another theme and found same issue, Here I have added my patch for another theme.

#19 @SergeyBiryukov
6 months ago

@viralsampat Thanks for the patch! It appears that these changes are already done as part of [55655] and [56045].

Please also note the message above the comment form:

This ticket was closed on a completed milestone.
If you have a bug or enhancement to report, please open a new ticket.

Note: See TracTickets for help on using tickets.