WordPress.org

Make WordPress Core

Opened 13 months ago

Closed 10 months ago

Last modified 10 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 13 months ago.
adding Yoda conditions
48455.1.patch (5.6 KB) - added by Marcio Zebedeu 13 months ago.
adding Yoda conditions
48455.2.patch (8.7 KB) - added by bookdude13 10 months ago.
Refreshing, added a couple more spots

Download all attachments as: .zip

Change History (17)

#2 @alishankhan
13 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
13 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
13 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
13 months ago

adding Yoda conditions

#6 @Marcio Zebedeu
13 months ago

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

@Marcio Zebedeu
13 months ago

adding Yoda conditions

#7 in reply to: ↑ 5 @alishankhan
13 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
13 months ago

@marcio-zebedeu
Please rebase your diff.

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

Replying to alishankhan:

@marcio-zebedeu
Please rebase your diff.

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

#10 @SergeyBiryukov
13 months ago

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

@bookdude13
10 months ago

Refreshing, added a couple more spots

#11 @bookdude13
10 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
10 months ago

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

#13 @SergeyBiryukov
10 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
10 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.