WordPress.org

Make WordPress Core

Opened 10 months ago

Closed 6 months ago

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

Download all attachments as: .zip

Change History (17)

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

adding Yoda conditions

#6 @Marcio Zebedeu
10 months ago

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

@Marcio Zebedeu
10 months ago

adding Yoda conditions

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

@marcio-zebedeu
Please rebase your diff.

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

Replying to alishankhan:

@marcio-zebedeu
Please rebase your diff.

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

#10 @SergeyBiryukov
10 months ago

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

@bookdude13
6 months ago

Refreshing, added a couple more spots

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

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

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