Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(debug-log): WP_DEBUG_LOG should be true by default #505

Merged
merged 1 commit into from
May 29, 2021
Merged

fix(debug-log): WP_DEBUG_LOG should be true by default #505

merged 1 commit into from
May 29, 2021

Conversation

QWp6t
Copy link
Member

@QWp6t QWp6t commented Apr 10, 2020

Setting WP_DEBUG_LOG to true will cause WP to automatically log debug output to <WP_CONTENT>/debug.log (so app/debug.log). This is a sensible default.

See: https://wordpress.org/support/article/debugging-in-wordpress/#wp_debug_log

@QWp6t QWp6t requested a review from tangrufus April 10, 2020 22:03
swalkinshaw
swalkinshaw approved these changes Apr 10, 2020
View reviewed changes
Copy link
Member

@swalkinshaw swalkinshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense 👍

@swalkinshaw
Copy link
Member

Actually.. this goes counter to what we decided in #499.

If the default in the config is false then this example .env should mirror that?

@QWp6t
Copy link
Member Author

QWp6t commented Apr 11, 2020

Yes, that's even better. Or remove the line entirely (or comment it out), which will fallback to default value of false. Just let me know what you prefer.

My reasoning for setting it to true was that we currently have non-false default value, so if we're going to have it non-false, then the safest non-false value is true instead of hard-coding a file path.

@swalkinshaw
Copy link
Member

We should just remove it imo. This doesn't even seem like a setting that will vary between environments? Since it primarily a development only setting.

@QWp6t
Copy link
Member Author

QWp6t commented Apr 11, 2020

In that case, perhaps we should move it to environments/development.php. 🤔

@swalkinshaw
Copy link
Member

I'm good with that since the two other debug related settings are already in there.

@austinpray I know our goal is to minimize env specific settings but I think this makes sense?

@austinpray
Copy link
Contributor

@austinpray I know our goal is to minimize env specific settings but I think this makes sense?

Yeah LGTM!

@tangrufus
Copy link
Member

tangrufus commented Apr 14, 2020 •
edited
Loading

Why we don't encourage setting WP_DEBUG_LOG to a path out of web root?
Non-trellis users might have example.com/app/debug.log exposed to the public.
See: https://www.google.com/search?hl=en&q=inurl%3Aapp%2Fdebug.log

@QWp6t
Copy link
Member Author

QWp6t commented Apr 14, 2020 •
edited
Loading

Latest commit changes a few things:

  • .env has option for specifying WP_DEBUG_LOG. (@tangrufus )
  • application.php sets WP_DEBUG_LOG to false
  • environments/development.php sets WP_DEBUG_LOG to value from .env or true to enable debug output during development

The last one there is probably what needs the most consideration. Do we want it true by default in development environment?

@swalkinshaw
Copy link
Member

@QWp6t think we just lost track of this, want to update it?

@QWp6t
Copy link
Member Author

QWp6t commented May 28, 2021

Will do. I think I just need to add that use function line

swalkinshaw
swalkinshaw approved these changes May 28, 2021
View reviewed changes
Copy link
Member

@swalkinshaw swalkinshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an improvement 👍

@QWp6t @swalkinshaw
Default to WP_DEBUG_LOG=true in development
1c017b0 
Setting WP_DEBUG_LOG to `true` will cause WP to automatically log debug
output to <WP_CONTENT>/debug.log (so app/debug.log) which is a sensible default.

This also comments out the old placeholder value in `.env.example` since
`true` is a better default.
@swalkinshaw swalkinshaw merged commit e940020 into master May 29, 2021
@swalkinshaw swalkinshaw deleted the db-log-true branch May 29, 2021 02:45
g000m added a commit to g000m/bedrock that referenced this pull request Jul 1, 2021
@g000m @swalkinshaw
@retlehs @QWp6t
merge upstream (#7)
0ac8182 
* Switch to GitHub actions (roots#597)

* Update build status badge

* Move disallow indexing mu-plugin to package (roots#521)

Co-authored-by: QWp6t <hi@qwp6t.me>
Co-authored-by: Scott Walkinshaw <scott.walkinshaw@gmail.com>

* Support .env.local config override (roots#594)

If `.env.local` exists, any values will override and take precedence
over those in `.env`.

This is meant for local development only.

* Default to `WP_DEBUG_LOG=true` in development (roots#505)

Setting WP_DEBUG_LOG to `true` will cause WP to automatically log debug
output to <WP_CONTENT>/debug.log (so app/debug.log) which is a sensible default.

This also comments out the old placeholder value in `.env.example` since
`true` is a better default.

Co-authored-by: Scott Walkinshaw <scott.walkinshaw@gmail.com>
Co-authored-by: Ben Word <ben@benword.com>
Co-authored-by: QWp6t <hi@qwp6t.me>
Co-authored-by: QWp6t <QWp6t@users.noreply.github.com>
@SteelWagstaff SteelWagstaff mentioned this pull request Aug 31, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andronocean andronocean andronocean left review comments

@swalkinshaw swalkinshaw swalkinshaw approved these changes

@tangrufus tangrufus Awaiting requested review from tangrufus

@austinpray austinpray Awaiting requested review from austinpray

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@QWp6t @swalkinshaw @austinpray @tangrufus @andronocean