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.

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

Add support for additional mu-plugins #611

Merged
merged 1 commit into from
Jun 20, 2017
Merged

Conversation

mjangda
Copy link
Member

@mjangda mjangda commented May 10, 2017

This change allows sites to add a client-mu-plugins folder (open to name change) in their VIP Go repo, which we load similar to mu-plugins. The client mu-plugins are loaded after the VIP Go mu-plugins are loaded ("enforced" by the filename prefix).

As part of this change, we will probably need a few others:

We will also probably need to re-create web containers for any sites that want to use this.

See #166

@mjangda
Copy link
Member Author

mjangda commented May 10, 2017

/cc @mikeselander

} while ( false !== $plugin );

closedir( $dh );
sort( $client_mu_plugins );
Copy link
Contributor

Choose a reason for hiding this comment

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

Presumably this is so files load in a predictable, alphabetical order instead of whatever order they happen to appear on the filesystem?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. Probably worth an inline comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added comment.


define( 'WPCOM_VIP_CLIENT_MU_PLUGIN_DIR', WP_CONTENT_DIR . '/client-mu-plugins' );

function wpcom_vip_load_client_mu_plugins() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to call this function somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Heh, good catch. Fixed.

Sites can add their own which are loaded after the VIP Go mu-plugins are
loaded ("enforced" by the filename prefix).
@mikeselander
Copy link

Thanks for writing this up and pinging me @mjangda - really appreciate the responsiveness on this request!

@CodeProKid
Copy link
Contributor

What's the status on this? Would love to start leveraging this.

@mjangda
Copy link
Member Author

mjangda commented Jun 15, 2017

Thanks for the ping @CodeProKid. This fell off our radar a bit but hoping to get it ready soon.

@mjangda
Copy link
Member Author

mjangda commented Jun 15, 2017

vip-go-skeleton update in Automattic/vip-go-skeleton#14

@mjangda
Copy link
Member Author

mjangda commented Jun 20, 2017

vipd has been updated to map the directory inside web containers.

@mjangda mjangda merged commit c1110aa into master Jun 20, 2017
@mjangda mjangda deleted the add/client-mu-plugins branch June 20, 2017 05:04
@mjangda
Copy link
Member Author

mjangda commented Jun 20, 2017

r94201-deploy

@mjangda
Copy link
Member Author

mjangda commented Jun 23, 2017

We're going to hold off on the vip-go-api update until we've added the folder to all existing repos/branches.

@fklein-lu
Copy link

fklein-lu commented Jun 26, 2017

I question the way that this feature was implemented:

  • Is there a need to define a global constant for the client MU plugins directory? The preference should be not to place more elements in the root namespace if it can be avoided.
  • Why define a function if it is invoked only once, immediately after its definition, and if the function contains protections against being called more than once? This code is not meant to be run more than once, so it shouldn't be placed in a reusable element.
  • By including the MU plugin files inside of a function, variables defined inside in these files are not placed in the global scope, but are restricted to the scope of the function. This behaviour differs from regular MU plugins.

The preferred approach should be to write this as a script:

  • No need for constants or functions.
  • Variables defined inside of MU plugins will be put in the right scope, without needing to do any of the awful things that wpcom_vip_load_plugin() does.
  • MU plugin files only get loaded once, so there's no need to protect against the client MU plugins being loaded multiple times.
  • You can use temporary variables, and as long as you unset them at the bottom of the script, they won't pollute the global namespace.

@mjangda
Copy link
Member Author

mjangda commented Jun 28, 2017

Thanks for the feedback, @fklein-lu, especially the point about scoping. The remaining points are somewhat subjective (e.g. putting the logic inside a function would allow us to more easily add tests) but will take them into consideration as we work to fix the scoping issue.

johnbillion added a commit to johnbillion/query-monitor that referenced this pull request May 21, 2018
pauarge added a commit that referenced this pull request Feb 24, 2022
5791cb356 Fix generating URL for invalid permalinks (#698)
7814bc406 Add Admin Bar filter (#691)
bd127a801 Adding tests for multiple WordPress versions (#689)
77658989e Don't return metadata on REST if no API key (#688)
a05640103 Fix API key empty checks (#686)
dc639a717 Adding additional wait on activation flow (#684)
20e99f75a Minor additions to the contributing file (#683)
e9a17d6ba Decrease default e2e timeout (#681)
e33bb25d5 Improving network support (#679)
35037250c Merge pull request #680 from Parsely/fix/failing-windows-tests
565a3fc6f repeated-metas.php: Change PHP_EOL to "\n"
2a1897726 Merge pull request #673 from Parsely/try/improve-wordpress-org-page
7da3113af Update plugin settings screenshots
662472d4b Merge branch 'try/improve-wordpress-org-page' of https://github.com/Parsely/wp-parsely into try/improve-wordpress-org-page
2114b3cc4 Revert "Update plugin settings screenshots"
35471e0bd Merge branch 'develop' into try/improve-wordpress-org-page
a582eebf0 Merge pull request #654 from Parsely/update/improve-docs
f68334989 Merge branch 'develop' into update/improve-docs
4899f5c5d Removing redundant EOL (#677)
d12ac8d5d Merge pull request #676 from Parsely/update/improve-function-comments
5c42bc581 Merge branch 'develop' into update/improve-function-comments
254469383 Update some outdated references in function comments
ebbfe504e Improving e2e reliability (#675)
872d95e59 README.md: Put visible tags first in order
30ced29c4 Merge branch 'try/improve-wordpress-org-page' of https://github.com/Parsely/wp-parsely into try/improve-wordpress-org-page
edd5c2aae Update plugin settings screenshots
f70123e72 Merge branch 'develop' into try/improve-wordpress-org-page
d3163ec0b Merge branch 'develop' into update/improve-docs
f2b8f2aa2 Support JS hooks for front-end actions (#650)
47a182a54 Docs: Improve readme
e0e64d07f README.md: Remove parse.ly tag
153770cba WINDOWS.md Fix code blocks within lists
1de133476 Update CONTRIBUTING.md
aab45d48a Merge branch 'update/improve-docs' of https://github.com/Parsely/wp-parsely into update/improve-docs
942a7bca0 CONTRIBUTING.md Fix code blocks within lists
7fa215685 README.md: Use tabs for code blocks
1fe0ee7a4 README.md: Move parse.ly tag to the end
424f31c5a Merge branch 'develop' into update/improve-docs
472e0462b Merge pull request #671 from Parsely/dependabot/npm_and_yarn/wordpress/scripts-21.0.2
ceb65389f Merge branch 'develop' into dependabot/npm_and_yarn/wordpress/scripts-21.0.2
22a5715bc Merge pull request #668 from Parsely/fix/save-plugin-settings-recrawl-section-hidden
1cd6e65de Bump @wordpress/scripts from 21.0.1 to 21.0.2
ff6a9112f Merge branch 'develop' into update/improve-docs
464805040 CONTRIBUTING.md: Improve tests section
c55563ad2 CONTRIBUTING.md: Improve rebuilding assets section
454c26faa Merge branch 'develop' into fix/save-plugin-settings-recrawl-section-hidden
d96ce815e Bump follow-redirects from 1.14.7 to 1.14.8 (#669)
a8d826266 Merge branch 'develop' into fix/save-plugin-settings-recrawl-section-hidden
c7e161331 Settings page: Remove E2E test for when recrawl settings are hidden
920d86177 Revert "Settings page: Fix E2E tests - attempt 4"
8bcdc96fb Settings page: Fix E2E tests - attempt 4
68ba7d297 Settings page: Fix E2E tests - attempt 3
835cb6ffa Settings page: Fix E2E tests - attempt 2
9f681dad2 Settings page: Fix E2E tests - attempt 1
e9258e22e Settings page: Add E2E test for when recrawl settings are hidden
c1042dcba Settings page: Update integration tests
2792d73fe Settings page: Save settings when recrawl section is hidden
f001479f2 Related Endpoint: Query args to backend & refactor for reuse (#649)
7944719d7 Fixing some type inconsistencies suggested by PHPStan (#657)
a454e83d4 Not installing env globally (#665)
5ac77252c Bump @wordpress/scripts from 20.0.2 to 21.0.1 (#664)
db443c915 Bump @wordpress/eslint-plugin from 10.0.0 to 10.0.1 (#661)
d117ec748 Bump @wordpress/babel-preset-default from 6.5.0 to 6.5.1 (#660)
9c39a8df8 Bump @wordpress/env from 4.2.0 to 4.2.1 (#663)
72c813c61 Bump @wordpress/e2e-test-utils from 6.0.0 to 6.0.1 (#659)
dc7f0dd2e Move recommended widget css to css folder (#658)
c91ef497d Moving recommended widget CSS to build system (#656)
77b0ddb42 Merge pull request #633 from Parsely/add/new-settings-post-type-tracking-ui
d37a4925d Use 'non-existent' instead of 'inexistent' (suggested by Gary Jones)
79d382e82 Remove js/css suffixes for admin setting asset handles
f5185a0fd utils.js: Fix comment for saveSettingsAndHardRefresh()
7403f9da0 WINDOWS.md: Replace tab with spaces
099fb38d8 WINDOWS.md: Add warning about PowerShell issue
e26b18865 Merge branch 'develop' into add/new-settings-post-type-tracking-ui
b82007dc8 Add E2E tests for "Track Post Types as" setting
8aab2180b Bring develop up to date with 3.1.1 release (#653)
db0c0a921 Fix recommended widget on WordPress 5.9 (#651)
b6da91158 Settings page: Update saving tracking values test
ada6d3a21 Settings page: Remove unneeded duplicate tracking detection
bee38cd37 Don't allow saving tracking settings with invalid values
e501a4fa2 Settings page: Don't allow saving tracking settings for inexistent post types
bd7539d40 Merge branch 'add/new-settings-post-type-tracking-ui' of https://github.com/Parsely/wp-parsely into add/new-settings-post-type-tracking-ui
689e40fe6 Settings page: Remove exclamation mark
5e41e1861 Merge branch 'develop' into add/new-settings-post-type-tracking-ui
98edcc882 Settings page: Harmonize help text with existing help texts
24fc2052e Add an endpoint to proxy Recommendations requests to the Parsely API (#611)
1fca9a0a1 Settings page: Fix help text capitalization
07a60443b get_tracking_values_for_display(): Add return type
f4780ad3d Merge branch 'add/new-settings-post-type-tracking-ui' of https://github.com/Parsely/wp-parsely into add/new-settings-post-type-tracking-ui
4a7aa4f73 Settings page: Adjust comments to satisfy code review
a060f7df4 Expose the wp-env script as-is in package.json (#648)
098837f17 Merge branch 'develop' into add/new-settings-post-type-tracking-ui
89c057b57 Improve reliability of E2E tests (#647)
e2ec11539 Merge branch 'add/new-settings-post-type-tracking-ui' of https://github.com/Parsely/wp-parsely into add/new-settings-post-type-tracking-ui
1269781e9 print_track_post_types_table(): Add legend tag to obey to SonarCloud
aa4eacd6d Merge branch 'develop' into add/new-settings-post-type-tracking-ui
dd7f5e9bc Merge branch 'add/new-settings-post-type-tracking-ui' of https://github.com/Parsely/wp-parsely into add/new-settings-post-type-tracking-ui
5b03c9e89 Simplify validate_options_post_type_tracking() function
04a955638 Correct since annotations to 3.1.0 (#646)
6302d728b Merge branch 'develop' into add/new-settings-post-type-tracking-ui
9e3e2f3c8 Cleaning up Scripts class (#644)
68a82b5bc Fix failing E2E tests
d8efcfae6 Remove unneeded blank line
f9eff7e86 Merge branch 'develop' into add/new-settings-post-type-tracking-ui
dae36d174 Settings page: Add built CSS file using webpack
018415458 Add Screen Options to toggle settings sections (#531)
4e170f78b Rename 'parsely_admin_settings_css' to 'wp-parsely-admin-style'
bdacbfc16 Merge branch 'add/new-settings-post-type-tracking-ui' of https://github.com/Parsely/wp-parsely into add/new-settings-post-type-tracking-ui
fe582d7b8 Settings page: Adjust 'Track Post Types as' table for small widths
00f05f9c8 Remove '...' from 'Track post type as...'
0381a2ba8 Add .wp-env.override.json to .gitignore & sort (#641)
29126813d Merge branch 'develop' into add/new-settings-post-type-tracking-ui
b5aec2bd5 Remove role="presentation" from 'Track Post Types as...' table
1c6d0d28a Settings page validation: Check for missing required arrays only when needed
99a654a51 Settings page: Add safety check during post tracking validation
6f6d2332e Settings Page: Implement load/save for 'Track Post Types as'
dd6f045e2 Settings page: Add new Track Post Type UI prototype
12f7df191 Use built version strings & deprecate Parsely::get_asset_cache_buster (#636)
56bba7f50 Unifying AMP and Google Web Stories implementation (#622)
fba4577e3 Bump @wordpress/scripts from 20.0.1 to 20.0.2 (#637)
afe4bd368 Merge pull request #620 from Parsely/fix/unhardcode-references-to-uncategorized
b58ccb97b Merge branch 'develop' into fix/unhardcode-references-to-uncategorized
733856696 Use WordPress scripts to export plugin (#634)
30fb81508 get_categories(): Remove unneeded null check
bdf8458d1 Remove unneeded 'use WP_Term'
f2522089a Simplify code for un-hardcoding 'Uncategorized' references
9d7dc7da2 Merge branch 'develop' into fix/unhardcode-references-to-uncategorized
97ea21eeb Merge branch 'develop' into add/new-settings-post-type-tracking-ui
ebd84b8d3 Build the admin settings page script and enqueue built version (#635)
4f7a9f1c2 Merge branch 'fix/unhardcode-references-to-uncategorized' of https://github.com/Parsely/wp-parsely into fix/unhardcode-references-to-uncategorized
a367f600a Refactor get_default_post_category_name() to return object
62045da2f Updating all npm dependencies (#632)
f73edf66b Merge branch 'develop' into fix/unhardcode-references-to-uncategorized
b6d7a8868 Switch to a theme that supports widgets (#631)
ae5d9e1b1 Settings: Fix function description
3c82946f3 get_default_post_category_name(): return '' when category array is empty
3669189df Attempt to fix failing test
8b9d1cc37 Parsely class: Add function to get default post category name
b58e85a7e Merge pull request #619 from Parsely/add/windows-md
efb063ee8 Add links to WINDOWS.md in CONTRIBUTING.md
5bcb712c0 Add WINDOWS.md
9c930ed7d Show settings link & API Key in Network Admin sites list (#583)
ca700dbd0 Adding support for Google Web Stories (#602)
296e92fe7 Merge pull request #613 from Parsely/update/contributing-md
7e8cf7306 Replace 'uncategorized' with default category name in main Parsely class
bd961a1fd Merge branch 'develop' into update/contributing-md
81da68c19 Allow user to choose logo using media library in plugin settings (#570)
3ff395b05 Update CONTRIBUTING.md
ccce9619d Update CONTRIBUTING.md
d40a324b0 CONTRIBUTING.md: Accept review changes
baa77e8fd Merge branch 'develop' into update/contributing-md
ae12e6695 Adding help for local development issues (#616)
9a3077a92 Remove plugin actions global variable (#615)
6938ce8d2 README.md: Change "meta data" to "metadata" (#614)
dab5af8ba CONTRIBUTING.md: Change structure for clarity and readability
32a7ac66c CONTRIBUTING.md: Improve some phrases for clarity
6a8914c77 Exclude some files & dirs from exported plugin archives (#612)
a6ac9ff7a Update follow-redirects package to the latest version (#610)
REVERT: 168e69711 Release 3.1.1 (#652)

git-subtree-dir: wp-parsely-3.1
git-subtree-split: 5791cb35679c2d79dbb66f94b1240d3a47ee80f0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants