Make WordPress Core

Opened 2 years ago

Closed 18 months ago

Last modified 14 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 2 years ago.
57397.2.patch (2.0 KB) - added by viralsampat 14 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)

@upadalavipul
2 years ago

#1 @sabernhardt
2 years ago

  • Component changed from General to Bundled Theme

#2 @hellofromTonya
22 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
22 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.


21 months ago
#4

  • Keywords has-patch added

#5 follow-up: @costdev
21 months ago

  • Version set to 5.0

#6 @mukesh27
21 months ago

Hi there!

Thanks for the ticket.

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

#7 @costdev
21 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
21 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 20 months ago by sabernhardt (previous) (diff)

@sabernhardt commented on PR #4225:


20 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
20 months ago

  • Keywords changes-requested added; commit removed

#12 @sabernhardt
20 months ago

  • Keywords changes-requested removed

#13 @mukesh27
18 months ago

  • Keywords commit added

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

Adding for commit consideration.

#14 @audrasjb
18 months ago

Self assigning for final review 👀

#15 @audrasjb
18 months ago

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

#16 @audrasjb
18 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
18 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
14 months ago

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

#19 @SergeyBiryukov
14 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.