Skip to content

use containing folder, just like guidelines suggest#1330

Closed
afragen wants to merge 18 commits intoWordPress:trunkfrom
afragen:hello-dolly
Closed

use containing folder, just like guidelines suggest#1330
afragen wants to merge 18 commits intoWordPress:trunkfrom
afragen:hello-dolly

Conversation

@afragen
Copy link
Copy Markdown
Member

@afragen afragen commented Jun 3, 2021

Currently Hello Dolly is installed as a single file plugin during a WP core installation. According to Plugin Handbook Best Practices, plugins should be in containing folders. https://developer.wordpress.org/plugins/plugin-basics/best-practices/#folder-structure

This is a simple PR to fix this issue with Hello Dolly. Having this means that things like r51064 are not necessary.

Related #49338

Trac ticket: https://core.trac.wordpress.org/ticket/53323


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@afragen afragen changed the base branch from master to trunk March 3, 2023 19:33
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 3, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props afragen, peterwilsoncc.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 4, 2024

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@peterwilsoncc
Copy link
Copy Markdown
Contributor

This will require an upgrade routine to check if hello.php is in the active list of plugins and replace it with hello-dolly/hello.php. During testing it deactivated the plugin when I checked out this branch.

Screenshot 2024-08-26 at 10 10 48 AM

For the last few years, Akismet has been automatically updated as part of the build process. This allows for a single source of truth and makes code changes to the plugin easier.

I think the approach may be helpful for Hello Dolly once it's moved in to a sub-folder as it will allow for a single source of truth for the code ratehr than having to update both the plugin and the copy in this repo. It would require a systems request but given there is prior art it should be simple enough to introduce.

@afragen
Copy link
Copy Markdown
Member Author

afragen commented Aug 26, 2024

Does it really matter if the single file hello.php plugin is disabled? If there is ever an update to Hello Dolly it would also disable as the update would install with a containing folder.

Since the plugin has no significant effect on the running of the site, I think a seamless transition is overkill.

@afragen
Copy link
Copy Markdown
Member Author

afragen commented Aug 26, 2024

@peterwilsoncc maybe I'm not understanding. If you check in the Playground a new WP installation will have Hello Dolly in a containing folder.

@peterwilsoncc
Copy link
Copy Markdown
Contributor

Given it's possible to have a seamless transition with a few lines of code, I think it's worth the effort.

$active_plugins = get_option( 'active_plugins', array() );
$hello_dolly = array_search( 'hello.php', $active_plugins );
if ( false !== $hello_dolly ) {
	$active_plugins[ $hello_dolly ] = 'hello-dolly/hello.php';
	update_option( 'active_plugins', $active_plugins );
}

I'm not sure what you mean by your second question, are you able to clarify?

@afragen
Copy link
Copy Markdown
Member Author

afragen commented Aug 26, 2024

If you have hello.php installed and there's an update, it will install hello-dolly/hello.php. The original plugin would be deleted and the update will be inactive.

@peterwilsoncc
Copy link
Copy Markdown
Contributor

The original plugin would be deleted and the update will be inactive.

@afragen I'm not seeing that, I modified the version number of hello.php and the plugin remained active once it was moved to hello-dolly/hello.php despite the original file being deleted. Maybe there is some snowflake code in core to handle this.

Even though it's a single file plugin, I think WP should handle moving the files as gracefully as it does when other files are relocated. The plugin is active on 700,000+ sites so even when the percentage of sites affected is small, the raw number is large.

@afragen
Copy link
Copy Markdown
Member Author

afragen commented Aug 26, 2024

I yield. Simply giving a counter argument 😉

@peterwilsoncc
Copy link
Copy Markdown
Contributor

Always happy to hear counter arguments. I can be incredibly wrong sometimes.

I'll check with meta (@dd32, I mean @dd32) about using an external to see if we can get a single source of truth for the code.

@afragen
Copy link
Copy Markdown
Member Author

afragen commented May 21, 2025

Clean merge and passing tests.

@github-actions
Copy link
Copy Markdown

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 60666
GitHub commit: 710675d

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.

@github-actions github-actions Bot closed this Aug 26, 2025
@afragen afragen deleted the hello-dolly branch August 26, 2025 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants