WordPress.org

Make WordPress Core

Opened 23 months ago

Closed 20 months ago

Last modified 19 months ago

#48455 closed defect (bug) (fixed)

Missing Yoda condition

Reported by: 1naveengiri Owned by: SergeyBiryukov
Milestone: 5.4 Priority: normal
Severity: normal Version: 5.3
Component: Administration Keywords: has-patch needs-testing
Focuses: coding-standards Cc:

Description

Attachments (3)

48455.patch (6.3 KB) - added by Marcio Zebedeu 23 months ago.
adding Yoda conditions
48455.1.patch (5.6 KB) - added by Marcio Zebedeu 23 months ago.
adding Yoda conditions
48455.2.patch (8.7 KB) - added by bookdude13 20 months ago.
Refreshing, added a couple more spots

Download all attachments as: .zip

Change History (17)

#2 @alishankhan
23 months ago

@1naveengiri Do we really need yoda conditions since it does not provide much advantage. But do take aways readiablity.

#4 in reply to: ↑ 3 ; follow-up: @alishankhan
23 months ago

Replying to 1naveengiri:

https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#yoda-conditions
Based on WP coding standards, Yes we need.

ohh.. Okay
Thanks

#5 in reply to: ↑ 4 ; follow-up: @Marcio Zebedeu
23 months ago

Replying to alishankhan:

Replying to 1naveengiri:

https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#yoda-conditions
Based on WP coding standards, Yes we need.

ohh.. Okay
Thanks

Is anyone working on this ticket?

@Marcio Zebedeu
23 months ago

adding Yoda conditions

#6 @Marcio Zebedeu
23 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

@Marcio Zebedeu
23 months ago

adding Yoda conditions

#7 in reply to: ↑ 5 @alishankhan
23 months ago

Replying to Marcio Zebedeu:

Replying to alishankhan:

Replying to 1naveengiri:

https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#yoda-conditions
Based on WP coding standards, Yes we need.

ohh.. Okay
Thanks

Is anyone working on this ticket?

yep I was, but no problem.Cheers!!

#8 follow-up: @alishankhan
23 months ago

@marcio-zebedeu
Please rebase your diff.

#9 in reply to: ↑ 8 @alishankhan
23 months ago

Replying to alishankhan:

@marcio-zebedeu
Please rebase your diff.

okay no need,
48455.1.patch​ is already rebased.

#10 @SergeyBiryukov
23 months ago

  • Component changed from General to Administration
  • Milestone changed from Awaiting Review to 5.4

@bookdude13
20 months ago

Refreshing, added a couple more spots

#11 @bookdude13
20 months ago

@alishankhan sorry if I'm hijacking this ticket, I wanted to help out a bit with 5.4 and this was a good candidate :)

Refreshed patch, caught a couple more cases where this was happening. Just needs some testing and another set of eyes to verify that the switched variables aren't changing the logic.

Note that the patch was made with git diff --no-ext-diff master > 48455.2.patch. If a different command is needed for SVN compatibility let me know and I can refresh.

#12 @SergeyBiryukov
20 months ago

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

#13 @SergeyBiryukov
20 months ago

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

In 47218:

Coding Standards: Use Yoda conditions in some wp-admin files.

Props subrataemfluence, marcio-zebedeu, bookdude13, 1naveengiri, alishankhan.
Fixes #44365, #48455.

#14 @SergeyBiryukov
19 months ago

In 47220:

Coding Standards: Correct the get_plugin_page_hook() check in wp-admin/admin.php.

The condition is meant to check for a non-empty string, however get_plugin_page_hook() can return null, in which case the strict check doesn't work as expected.

Follow-up to [47218].

See #48455, #49222.

Note: See TracTickets for help on using tickets.