Closed kleerwater closed 7 years ago
I'm going to accept the pull request, though I'll tweak the new test a bit to give better error messages.
Thanks for the contribution. Sorry for the delay in the review.
FYI here are the polishes:
https://github.com/zorzella/guiceberry/commit/aee23d55bb5b3a17ced1a0fe67e48c3f0c25a40f
Thanks for reviewing and pulling it.
On Thu, Jan 12, 2017 at 11:53 AM zorzella notifications@github.com wrote:
FYI here are the polishes:
aee23d5 https://github.com/zorzella/guiceberry/commit/aee23d55bb5b3a17ced1a0fe67e48c3f0c25a40f
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/zorzella/guiceberry/pull/32#issuecomment-272264726, or mute the thread https://github.com/notifications/unsubscribe-auth/AFIwFiwurtu8Z270BKb3kzsnmsC871GVks5rRoSbgaJpZM4KH98H .
Note: while trying to deploy the new GuiceBerry release into our larger codebase I encountered an issue with this change that I did not anticipate. I still plan on keeping this change (unless I find it intractable to fix all issues caused by it), but here's an amended portion of the release notes to cover that:
Ok, as discussed offline, the problem was actually just with things used by the TestWrapper, not as broad as I originally thought. It also is fixable, and I did just that:
https://github.com/zorzella/guiceberry/commit/882ffd43e7fc400c8ef72e17878bbacdaf49bf55
It's painful to start servers only to find that there is an unsatisfied binding in the test