Skip to content

Leave wp-content/plugins and wp-content/themes directories when using --skip-content#230

Merged
schlessera merged 1 commit intomainfrom
228-create-wp-content-plugins-themes
Apr 27, 2023
Merged

Leave wp-content/plugins and wp-content/themes directories when using --skip-content#230
schlessera merged 1 commit intomainfrom
228-create-wp-content-plugins-themes

Conversation

@danielbachhuber
Copy link
Member

@danielbachhuber danielbachhuber commented Apr 24, 2023 •
edited
Loading

Creates the wp-content/plugins and wp-content/themes directories (and their corresponding index.php files) when using wp core download --skip-content, instead of ignoring them.

While these directories were intentionally excluded in the original pull request...

  • Installing a WordPress plugin in the admin fails if the directory doesn't exist.
  • The intent of the original pull request was to exclude default plugins and themes.
  • We're creating the wp-content directory anyway (pull request), so I don't think it's rational to create wp-content but not wp-content/plugins.

I'm not sure if we should consider this a breaking change, or a bug with the original implementation.

Fixes #228

@danielbachhuber danielbachhuber added command:core-download Related to 'core download' command command:core Related to 'core' command labels Apr 24, 2023
@danielbachhuber danielbachhuber self-assigned this Apr 24, 2023
@danielbachhuber danielbachhuber requested a review from a team as a code owner April 24, 2023 22:40
@danielbachhuber danielbachhuber mentioned this pull request Apr 24, 2023
Closed
@swissspidy
Copy link
Member

swissspidy commented Apr 26, 2023

What about other directories like wp-content/upgrade and wp-content/languages, are these also affected?

@danielbachhuber
Copy link
Member Author

danielbachhuber commented Apr 26, 2023

@swissspidy Neither of those directories are affected. Here's where we unwound accidental deletion of wp-content/languages: #212

@schlessera
Copy link
Member

schlessera commented Apr 27, 2023

I'd consider this a bug fix. We were too aggressive with the removal and caused issues, and as you said, the original intent was not to remove the folders, but rather to not include the default extensions. So I'd say this is not a breaking change.

@schlessera schlessera added this to the 2.1.11 milestone Apr 27, 2023
@schlessera schlessera merged commit e3212a3 into main Apr 27, 2023
@schlessera schlessera deleted the 228-create-wp-content-plugins-themes branch April 27, 2023 09:50
@maximejobin
Copy link

maximejobin commented May 4, 2023

Sorry for being late to the party. I would also argue this is a bug fix... that can break code.

My main argument is : installing WP with CLI with the --skip-content flag makes a user unable to install a theme or plugin if it's not done, at first, with CLI. So if you just install WP with CLI then never use CLI again, you are left with an unusable installation regarding the addition of themes and plugins.

@danielbachhuber
Copy link
Member Author

danielbachhuber commented May 4, 2023

My main argument is : installing WP with CLI with the --skip-content flag makes a user unable to install a theme or plugin if it's not done, at first, with CLI. So if you just install WP with CLI then never use CLI again, you are left with an unusable installation regarding the addition of themes and plugins.

I think this is a reasonable argument! Thanks for sharing, @maximejobin

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

Reviewers

@schlessera schlessera schlessera approved these changes

@swissspidy swissspidy swissspidy approved these changes

Assignees

@danielbachhuber danielbachhuber

Labels

command:core Related to 'core' command command:core-download Related to 'core download' command

Projects

None yet

Milestone

2.1.11

Development

Successfully merging this pull request may close these issues.

The --skip-content flag does not create folders inside wp-content

4 participants

@danielbachhuber @swissspidy @schlessera @maximejobin