Skip to content
This repository has been archived by the owner on Jul 31, 2018. It is now read-only.

Plugin integration #9

Merged
merged 5 commits into from
Oct 2, 2013
Merged

Plugin integration #9

merged 5 commits into from
Oct 2, 2013

Conversation

jsilvestre
Copy link
Contributor

  • plugin usage is now:
    Configuration:
#./server/config.coffee
plugins: [
    'an-americano-plugin'
]

Usage:

americano = require 'americano'
americano.pluginProperty()

There is no detection of eventual collision, maybe I can add a warning message if a collision is detected (would only require to rewrite the extends function, no big deal) ?

  • we can specify absolute paths for plugins (to ease plugin development/debugging)
  • in the "configure" method of plugins, you can use "this" as americano instance

Before merging, I think we could discuss the plugin configuration so I can add it to the PR.
To keep things simple this is what I propose:

plugins: [
    'plugin-name': 
        'configKey1': 'config value 1'
        'configKey2': 'config value 2'
    ]

Then we could change the "configure" plugin methods by adding the options:

coffeescript
module.exports.configure = (root, app, options, callback) ->

Tell me what you think and I will add the proper changes.

@frankrousseau
Copy link
Contributor

I'm ok with your change if the current way still works.

plugins: [
'plugin-name',
]

@jsilvestre
Copy link
Contributor Author

I haven't found out a way to add plugin options without breaking backward compatibility, so I guess you can merge the PR without that change.

@jsilvestre
Copy link
Contributor Author

I have changed loading order to avoid the following case:

  • routes are loaded, they load controllers
  • controllers load models
  • models do a "americano.getModel" (americano-cozy example)
    => plugins are not loaded yet, getModel does not exist.

frankrousseau pushed a commit that referenced this pull request Oct 2, 2013
@frankrousseau frankrousseau merged commit 4ab450c into cozy:master Oct 2, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants