WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 5 months ago

#11598 reopened enhancement

code improvements in wp_:dashboard_plugins_output()

Reported by: hakre Owned by:
Milestone: Future Release 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 4 years ago.
11598.2.patch (5.1 KB) - added by hakre 4 years ago.
Next Iteration

Download all attachments as: .zip

Change History (12)

hakre4 years ago

comment:1 hakre4 years ago

  • Keywords tested added

Tested against current trunk, Plugin listing works flawlessly.

comment:2 hakre4 years ago

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

comment:3 follow-up: dd324 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

comment:4 Denis-de-Bernardy4 years ago

  • Keywords needs-patch added; has-patch removed

comment:5 nacin4 years ago

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

comment:6 in reply to: ↑ 3 hakre4 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.

hakre4 years ago

Next Iteration

comment:7 hakre4 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.

comment:8 nacin3 years ago

  • Milestone changed from Awaiting Triage to Future Release

comment:9 c3mdigital8 months 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/

comment:10 markoheijnen5 months 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.

Note: See TracTickets for help on using tickets.