WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 11 months ago

#11598 closed enhancement (wontfix)

code improvements in wp_:dashboard_plugins_output()

Reported by: hakre Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.9
Component: General Keywords: needs-patch
Focuses: Cc:

Description

Smaller code changes to improve the function. Stumbeled over this while digging into #11597 and #11518.

Attachments (2)

11598.patch (2.6 KB) - added by hakre 7 years ago.
11598.2.patch (5.1 KB) - added by hakre 6 years ago.
Next Iteration

Download all attachments as: .zip

Change History (13)

@hakre
7 years ago

#1 @hakre
7 years ago

  • Keywords tested added

Tested against current trunk, Plugin listing works flawlessly.

#2 @hakre
7 years ago

Still ready to commit. Can a commiter please take care?

#3 follow-up: @dd32
7 years ago

  • Keywords tested removed

Does that do {} while(false); loop work? That will only ever run once from looking at the code.

Continue will just skip the rest of the loop and go straight to the conditional.

Best to leave it as a while(true) { break;} , Theres no need for the do syntax which is out of line with the rest of WordPress

#4 @Denis-de-Bernardy
7 years ago

  • Keywords needs-patch added; has-patch removed

#5 @nacin
6 years ago

  • Milestone changed from 3.0 to 3.1
  • Type changed from defect (bug) to enhancement

#6 in reply to: ↑ 3 @hakre
6 years ago

Replying to dd32:

Does that do {} while(false); loop work? That will only ever run once from looking at the code.

Yes/Yes: Yes, it does work. Second yes is clear I guess. Let me know if not.

Best to leave it as a while(true) { break;} , Theres no need for the do syntax which is out of line with the rest of WordPress

Will adopt the patch accordingly to that feedback.

@hakre
6 years ago

Next Iteration

#7 @hakre
6 years ago

  • Keywords has-patch added; needs-patch removed
  • Changed the patch according to dd32 feedback regarding the do/while clause.
  • Used in_array() to check for installed slugs instead of the reset/foreach combo[sic!] that is currently in there. Let me know if that's ok or not (that was with substr() compare but I assumed that this implicated problems as well).
  • Description filter is put next to the other filters.
  • Removed unnecessary isset() check after the while(true) loop.
  • Added comments
  • Formatted the code according to the styleguide

I have this running on my testbed against current trunk with no probs.

#8 @nacin
6 years ago

  • Milestone changed from Awaiting Triage to Future Release

#9 @c3mdigital
3 years ago

  • Resolution set to maybelater
  • Status changed from new to closed

Refactoring tickets should have roper justification and clear rationale, performance benchmarks and unit tests, See: http://make.wordpress.org/core/2011/03/23/code-refactoring/

#10 @markoheijnen
3 years ago

  • Keywords needs-patch added; has-patch removed
  • Resolution maybelater deleted
  • Status changed from closed to reopened

Due to the changes in 3.8 I was wanted close this ticket as wint fix. Looking at the code it it's confusing. It has obviously something to do with core refactoring but I think that function need to be looked at in 3.9. Specially since we simplified it now.

#11 @wonderboymusic
11 months ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

No movement in 5 years and none since 3.8 overhaul

Note: See TracTickets for help on using tickets.