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

Optional config and prefixed producer config key #11

Merged
merged 2 commits into from
Oct 1, 2015

Conversation

codeliner
Copy link
Member

This PR provides some improvements for the factory:

  • application config is optional as the producer has smart defaults
  • moved config location into a protected method and removed the final keyword from the factory so that a userland implementation can extend the factory and use for example interop-config to load config from a custom path
  • changed the config package name from producer to zeromq_producer as we have various producers available for prooph/service-bus which might be used in parallel

@codeliner codeliner added this to the 0.2 Release milestone Oct 1, 2015
@codeliner
Copy link
Member Author

ping @bweston92 please review and merge if you are fine with the changes

@bweston92
Copy link
Member

Ok it changes the name of the configuration value which is a BC break but I think it is only me who has used it anyway.

bweston92 pushed a commit that referenced this pull request Oct 1, 2015
Optional config and prefixed producer config key
@bweston92 bweston92 merged commit 1871239 into prooph:master Oct 1, 2015
@codeliner
Copy link
Member Author

yeah, we are not stable yet so BC breaks are ok. But I'll add a note in the release.

@sandrokeil
Copy link
Member

@codeliner @bweston92 It looks like the interop-config HasDefaultOptions Interface is a perfect match here. I've planned a new release next week and can bring a pull request if you're interest in this.

@bweston92
Copy link
Member

@sandrokeil that would be great, that means we wouldn't need the container at all!

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

Successfully merging this pull request may close these issues.

3 participants