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

Feature/move plugins #31

Merged
merged 6 commits into from
Oct 11, 2016
Merged

Feature/move plugins #31

merged 6 commits into from
Oct 11, 2016

Conversation

vutran
Copy link
Member

@vutran vutran commented Oct 8, 2016

Moving plugins-related functions into a separate module so it is more maintainable.

The new plugins module has a new method enable(), and disable() which encapsulates adding/removing the plugin from ~/.dext/config.json to enable resolving DextApp/dext#64. This method is accessible via:

  • api.plugins.enable(plugin)
  • api.plugins.disable(plugin)

In addition, createSymLink(), and removeSymLink() will now enable and disable the plugin. This resolves vutran/dext-cli#21

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 045ac28 on feature/move-plugins into 0834742 on develop.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 491d39f on feature/move-plugins into 0834742 on develop.

exports.ERR_MODULE_NOT_FOUND = 'ERR_MODULE_NOT_FOUND';
exports.ERR_MODULE_NOT_ENABLED = 'ERR_MODULE_NOT_ENABLED';
Copy link
Member

Choose a reason for hiding this comment

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

Why not ERR_MODULE_DISABLED?

* @param {String} plugin - The plugin name
* @return {Boolean}
*/
const isEnabled = plugin => getAll().indexOf(plugin) > -1;
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be simplified

Boolean(~getAll().indexOf(plugin))

Copy link
Member Author

Choose a reason for hiding this comment

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

Boolean() seems unnecessary but NOT op LGTM. Will add this soon.

Copy link
Member Author

Choose a reason for hiding this comment

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

@adnasa Not sure if you saw this.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 8c7cfd5 on feature/move-plugins into 0834742 on develop.

@vutran vutran merged commit b3d7350 into develop Oct 11, 2016
@vutran vutran deleted the feature/move-plugins branch October 11, 2016 23:14
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

3 participants