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

Make email/slack strategy and their dependencies optional. #37

Closed
wants to merge 3 commits into from

Conversation

LostKobrakai
Copy link

This is a breaking change, because dependencies for the email/slack strategy will no longer be installed with Ravenx, but each user does need to install those dependencies manually and add the strategy to his config.

On the flip side ravenx does will not install any unnecessary dependencies a user might not need. This will (at least partially) solve #28.

@odarriba
Copy link
Member

odarriba commented Feb 12, 2018

Hi @LostKobrakai

I'm sorry about the delay on this.

I cannot accept your merge because it will really generate issues to existing users, causing errors and potentially production issues when they don't realise that some dependencies are missing and the code try to find a module that is not there.

I know that that's the responsibility of each one, but I know that this will cause troubles out there.

But! There is a plan to separate the different strategies into other modules, and that's the way we are going to follow for this, preparing the next major version removing those dependencies, and also the code that needs them, into different packages.

Even not being able to merge this PR, thank you for your effort, every piece of community effort is valuable! ❤️

@odarriba odarriba closed this Feb 12, 2018
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