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

Extending plugins functionality (two extra functions) #1194

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

michalmanko
Copy link

Now plugin can have two extra functions:

  • initializeDb() - a Databank object can be passed to a plugin directly
  • configureApp() - this method is executed much earlier than initializeApp() and allows to configure the app (e.g. add a middleware)

Now plugin can have two extra functions:
- initializeDb() - a Databank object can be passed to a plugin directly
- configureApp() - this method is executed much earlier than initializeApp() and allows to configure the app (e.g. add a middleware)
@strugee
Copy link
Member

strugee commented Sep 3, 2016

@michalmanko thanks for sending this PR.

Unfortunately I'm uncomfortable merging this, at least at the present time. Plugins aren't really supported right now - Evan has indicated that they may go away in a future pump.io version (see #588 where they were originally implemented), and I'm inclined to rip out the code until we're ready to support them. If we end up doing that, merging this doesn't really make a whole lot of sense.

To elaborate, right now we have a lot of work ahead of us upgrading core dependencies - see for example the work going on in the express-3.x branch. The way that plugins are implemented, the libraries used internally by the application essentially become part of the public plugin API. If we choose to officially support plugins, each dependency upgrade would trigger a bump of the major version number of pump.io. Otherwise, we wouldn't be respecting semver by causing potential plugin breakage.

In any case, I'll bring this up at the next monthly meeting.

@strugee
Copy link
Member

strugee commented Sep 3, 2016

Note also that plugins are currently undocumented.

@strugee
Copy link
Member

strugee commented Oct 31, 2016

Hey @michalmanko - still interested in getting this into pump.io core?

We decided during the meeting before last not to drop plugin functionality just yet (although we do plan to eventually replace it with something that doesn't have the problems I've described above). Hence, I have no issues merging this PR - sorry I forgot about it until now!

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.

None yet

2 participants