-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Plugin API Refactor: Filter and Theme Helpers #931
Conversation
return plugins.init(this, pluginsToLoad).then(function (loadedPlugins) { | ||
if (!_.isArray(pluginsToLoad)) { | ||
// Load the default plugins from settings if not passed | ||
return models.Settings.read("activePlugins").then(function (activePluginsSetting) { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
issue TryGhost#932 - handles outputting assets for the admin and theme which have a version query string
Updated Changed activePlugins default to |
} | ||
|
||
// Recursively call ourself again with the active plugins | ||
return self.initPlugins(server, settingValue); |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
@jgable: great work on this PR, BTW. I hope it gets merged, because I am working on an async plugin. I have 1 other small point: I wonder if initPlugins is the right place to set ghost.server. It seems like the code in initPlugins can be used to initialize newly activated plugins while the server is running, so it might run more than once. I can understand that plugins might need ghost.server, but perhaps it can be set directly in server.js? |
- Refactor doFilter to allow returning a promise from a filter handler and to also return a promise itself - Move the logic out of the registerThemeHelper calls and into their own methods so we could test them in isolation. - Assign the server to the ghost instance so the initPlugins method can get access to it.
Updated Rebased on latest from master. Included adding some new helpers and refactoring a couple tests to match the new style. Also, refactored the initPlugins per @frysbe suggestions. Specifically, not calling itself recursively and having server.js attach the express instance onto the ghost object itself. |
ghost.server = server; | ||
|
||
// Initialize plugins then start the server | ||
ghost.initPlugins().then(function () { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
Manually merged along with error handling improvements. |
@jgable Thanks again for your work! I'm happy this has been merged. |
and to also return a promise itself
we could test them in isolation.
ghost.init() to server startup.
This should cover the first 3 checkboxes in #769 for the plugin architecture.