Skip to content

Script debug when version controlled#12457

Closed
ellatrix wants to merge 1 commit intomasterfrom
try/script-debug
Closed

Script debug when version controlled#12457
ellatrix wants to merge 1 commit intomasterfrom
try/script-debug

Conversation

@ellatrix
Copy link
Copy Markdown
Member

Description

When I setup Gutenberg for local development, I'd expect it to load unminified files.

How has this been tested?

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@ellatrix ellatrix requested a review from a team November 30, 2018 10:26
function gutenberg_script_debug() {
return (
( defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG ) ||
file_exists( gutenberg_dir_path() . '.git' )
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if, in the course of development, I do want to test with minified souces? Switching on the existence of .git seems like a very inflexible method.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The same is true if we set it in the Docker config: you'd need to edit the config.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

An environment variable seems the best option here; why not have MINIFY_JS, SCRIPT_DEBUG, or something else, be an env var we could check for?

Copy link
Copy Markdown
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

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

I agree with @mcsf that checking on .git is a bit too inflexible, but using an env var would work well here I think.

function gutenberg_script_debug() {
return (
( defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG ) ||
file_exists( gutenberg_dir_path() . '.git' )
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

An environment variable seems the best option here; why not have MINIFY_JS, SCRIPT_DEBUG, or something else, be an env var we could check for?

@gziolo gziolo added the [Type] Build Tooling Issues or PRs related to build tooling label Jan 31, 2019
@gziolo gziolo added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Mar 8, 2019
@gziolo
Copy link
Copy Markdown
Member

gziolo commented Mar 8, 2019

Is there any way to move it forward? This PR got out of date but it would be awesome to find a way to enable this flag through Docker.

@pento
Copy link
Copy Markdown
Member

pento commented Mar 9, 2019

You can do it by adding these lines to install-wordpress.sh:

docker-compose $DOCKER_COMPOSE_FILE_OPTIONS run --rm $CLI config set WP_DEBUG true --raw --type=constant
docker-compose $DOCKER_COMPOSE_FILE_OPTIONS run --rm $CLI config set SCRIPT_DEBUG true --raw --type=constant
docker-compose $DOCKER_COMPOSE_FILE_OPTIONS run --rm $CLI config set WP_DEBUG_DISPLAY true --raw --type=constant

Location in the file doesn't matter, they just need wp-config.php to exist, which the WordPress container automatically creates.

@gziolo
Copy link
Copy Markdown
Member

gziolo commented Mar 11, 2019

@pento this is something @aduth figured out in one of the recent PRs working on something unrelated. I will close this PR and open another one which offers ENV variable to to flip those constant from CLI.

@gziolo gziolo closed this Mar 11, 2019
@gziolo gziolo deleted the try/script-debug branch March 11, 2019 08:38
@gziolo
Copy link
Copy Markdown
Member

gziolo commented Mar 11, 2019

I opened #14371 where I want to provide an option to override site constants through env variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. [Type] Build Tooling Issues or PRs related to build tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants