You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2021/11/03 18:31:23 UTC
[GitHub] [bookkeeper] Vanlightly opened a new pull request #2876: There should only be a single BookieImpl constructor
Vanlightly opened a new pull request #2876:
URL: https://github.com/apache/bookkeeper/pull/2876
### Motivation
To have DI on the bookie, it's better to have a single constructor
that takes all the injected implementations. With a single
constructor, there's only one place modify in production code when we
want to inject something, rather than having the bookie need to know
how to create a "default" version (which often breaks encapsulation).
If we need convenience constructors for tests, they should live in the
tests.
### Changes
The principle changes are:
- remove the BookieImpl constructor which takes only a ServerConfiguration and chooses default values for the other ijectable dependencies.
- add a TestBookieImpl for use in tests which has a constructor that matches the one removed from BookieImpl.
This is one of many preliminary PRs with refactorings required for the "BP-46 Running without journal" work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Vanlightly commented on pull request #2876: There should only be a single BookieImpl constructor
Posted by GitBox <gi...@apache.org>.
Vanlightly commented on pull request #2876:
URL: https://github.com/apache/bookkeeper/pull/2876#issuecomment-960838472
rerun failure checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] ivankelly merged pull request #2876: There should only be a single BookieImpl constructor
Posted by GitBox <gi...@apache.org>.
ivankelly merged pull request #2876:
URL: https://github.com/apache/bookkeeper/pull/2876
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on pull request #2876: There should only be a single BookieImpl constructor
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2876:
URL: https://github.com/apache/bookkeeper/pull/2876#issuecomment-960552139
PTAL to failed tests on CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Vanlightly commented on pull request #2876: There should only be a single BookieImpl constructor
Posted by GitBox <gi...@apache.org>.
Vanlightly commented on pull request #2876:
URL: https://github.com/apache/bookkeeper/pull/2876#issuecomment-961028040
The failing test has been fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org