Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#46692 closed defect (bug) (fixed)

Site Health: WP_DEBUG_LOG just shown as Enabled when it actually contains a path

Reported by: knutsp's profile knutsp Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.2 Priority: normal
Severity: minor Version: 5.2
Component: Site Health Keywords: site-health has-patch commit
Focuses: Cc:

Description

Under tab Site Info, paragraph WordPress Constants.

Suggestion: When defined, but not identical to true, and not identical to false, display it (the path) as with the other path constants above it.

Related: #46689

Attachments (3)

46692.diff (752 bytes) - added by xkon 6 years ago.
46692.2.diff (1.1 KB) - added by xkon 6 years ago.
46692.3.diff (1.0 KB) - added by xkon 6 years ago.

Download all attachments as: .zip

Change History (13)

#1 @xkon
6 years ago

46692.diff adds an extra check and displays the value of WP_DEBUG_LOG as well in WordPress Constants section.

Not really sure if we can stretch the ternary there like that so I'd like an opinion. We can always move the check elsewhere for better readability if needed :) .

Last edited 6 years ago by xkon (previous) (diff)

@xkon
6 years ago

#2 @xkon
6 years ago

  • Keywords has-patch 2nd-opinion added

#3 @Clorith
6 years ago

  • Keywords has-patch 2nd-opinion removed

I agree that it's stretching the ternary's a bit deep at this point, let's move the logic down into an if statement or two after the large variable declaration.

@xkon
6 years ago

#4 @xkon
6 years ago

  • Keywords has-patch added

46692.2.diff replaces the ternary that checks if/else and moves it before the debug info array.

I think that since we already had some vars passed into the array at the very start ( for updates etc ) it made more sense to me this way than adding it into the array later on ( it also cleanly preserves the original placement ).

#5 @Clorith
6 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 5.2

Sounds reasonable, before or after makes no difference in this scenario.

#6 @ocean90
6 years ago

  • Keywords needs-refresh added; commit removed

WP_DEBUG_LOG is always defined (false by default) so that check can be simplified a bit.

@xkon
6 years ago

#7 @xkon
6 years ago

  • Keywords needs-refresh removed

46692.3.diff simplifies the if as @ocean90 mentioned.

I think we can simplify other ternary if/else as well since we check for more constants that are always defined as well. I'll create a new ticket for that :) .

#8 @SergeyBiryukov
6 years ago

  • Keywords commit added

#9 @SergeyBiryukov
6 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 45079:

Site Health: If WP_DEBUG_LOG contains a file path, display it on Site Info tab.

Props xkon, knutsp, ocean90.
Fixes #46692.

#10 @spacedmonkey
6 years ago

  • Component changed from Administration to Site Health
Note: See TracTickets for help on using tickets.