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

permit specifying haraka plugins w/o haraka-plugin- prefix #1647

Merged
merged 1 commit into from Oct 9, 2016

Conversation

Projects
None yet
2 participants
@msimerson
Member

msimerson commented Oct 8, 2016

Changes proposed in this pull request:

  • permit specifying haraka plugins w/o haraka-plugin- prefix
  • in config/plugins
  • in resultstore
permit specifying haraka plugins w/o haraka-plugin- prefix
- in config/plugins

- in resultstore
@msimerson

This comment has been minimized.

Show comment
Hide comment
@msimerson

msimerson Oct 8, 2016

Member

There's an implicit question in this PR, mostly for @baudehlo . Which is, how are you loading haraka-plugin-outbound-rate-limit? I presume the only way it can work (without the change in this PR) is that you are specifying 'haraka-plugin-outbound-rate-limit' in your config/plugins file. Which is redundant and kinda ugly, because then plugin.name is haraka-plugin-outbound-rate-limit instead of outbound-rate-limit.

I prefer this solution, of specifying only the plugin name, and having the haraka-plugin- prefix automatically prepended in the file search. That way plugin.name is set properly by default. If we remove the 3rd entry (which seems kinda not all that helpful), it would also enforce the haraka-plugin namespace prefix for plugins.

Thoughts?

Member

msimerson commented Oct 8, 2016

There's an implicit question in this PR, mostly for @baudehlo . Which is, how are you loading haraka-plugin-outbound-rate-limit? I presume the only way it can work (without the change in this PR) is that you are specifying 'haraka-plugin-outbound-rate-limit' in your config/plugins file. Which is redundant and kinda ugly, because then plugin.name is haraka-plugin-outbound-rate-limit instead of outbound-rate-limit.

I prefer this solution, of specifying only the plugin name, and having the haraka-plugin- prefix automatically prepended in the file search. That way plugin.name is set properly by default. If we remove the 3rd entry (which seems kinda not all that helpful), it would also enforce the haraka-plugin namespace prefix for plugins.

Thoughts?

@baudehlo

This comment has been minimized.

Show comment
Hide comment
@baudehlo

baudehlo Oct 9, 2016

Collaborator

Fine by me. I am not using it. I wrote it as a proof of concept. And I don't know anyone using it anyway.

On Oct 8, 2016, at 6:40 PM, Matt Simerson notifications@github.com wrote:

There's an implicit question in this PR, mostly for @baudehlo . Which is, how are you loading haraka-plugin-outbound-rate-limit? I presume the only way it can work (without the change in this PR) is that you are specifying 'haraka-plugin-outbound-rate-limit' in your config/plugins file. Which is redundant and kinda ugly, because then plugin.name is haraka-plugin-outbound-rate-limit instead of outbound-rate-limit.

I prefer this solution, of specifying only the plugin name, and having the haraka-plugin- prefix automatically prepended in the file search. That way plugin.name is set properly by default. If we remove the 3rd entry (which seems kinda not all that helpful), it would also enforce the haraka-plugin namespace prefix for plugins.

Thoughts?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

Collaborator

baudehlo commented Oct 9, 2016

Fine by me. I am not using it. I wrote it as a proof of concept. And I don't know anyone using it anyway.

On Oct 8, 2016, at 6:40 PM, Matt Simerson notifications@github.com wrote:

There's an implicit question in this PR, mostly for @baudehlo . Which is, how are you loading haraka-plugin-outbound-rate-limit? I presume the only way it can work (without the change in this PR) is that you are specifying 'haraka-plugin-outbound-rate-limit' in your config/plugins file. Which is redundant and kinda ugly, because then plugin.name is haraka-plugin-outbound-rate-limit instead of outbound-rate-limit.

I prefer this solution, of specifying only the plugin name, and having the haraka-plugin- prefix automatically prepended in the file search. That way plugin.name is set properly by default. If we remove the 3rd entry (which seems kinda not all that helpful), it would also enforce the haraka-plugin namespace prefix for plugins.

Thoughts?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@msimerson

This comment has been minimized.

Show comment
Hide comment
@msimerson

msimerson Oct 9, 2016

Member

And I don't know anyone using it anyway.

AFAIK, the watch plugin is the only such animal. So I'll package it up as an npm module and then path #3 in that list can go away.

Member

msimerson commented Oct 9, 2016

And I don't know anyone using it anyway.

AFAIK, the watch plugin is the only such animal. So I'll package it up as an npm module and then path #3 in that list can go away.

@msimerson msimerson merged commit e5460dd into haraka:master Oct 9, 2016

3 checks passed

codecov/patch Coverage not affected when comparing d9252c1...0a3cda5
Details
codecov/project 34.41% (+0.00%) compared to d9252c1
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@msimerson msimerson deleted the msimerson:haraka-plugin-path branch Oct 9, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment