-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat(configFactory): be able to access junit-context during config of the server #17
feat(configFactory): be able to access junit-context during config of the server #17
Conversation
yea, looks good, thx for the PR, please continue |
If this is ok for you, could you say to me what you would like me to add to this PR to be accepted? |
253f708
to
bc740c5
Compare
I've added one unit test about this new feature. If this is enough from your point of view, you can merge it 👍 |
@@ -14,9 +15,9 @@ | |||
*/ | |||
class WiremockFactory { | |||
|
|||
public WireMockServer createServer(final Wiremock mockedServer) { | |||
public WireMockServer createServer(final Wiremock mockedServer, ExtensionContext extensionContext) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can I ask you to keep the previous signature as well? To not break the binary compatibility. Thus you can avoid changes on the tests, adding nulls where the context should be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, j8 is the main target for now still, but thx for pointing
please rebase on master, fixed an issue with the build |
bc740c5
to
42c1796
Compare
Codecov Report
@@ Coverage Diff @@
## master #17 +/- ##
============================================
+ Coverage 93.75% 94.02% +0.27%
- Complexity 24 28 +4
============================================
Files 7 7
Lines 64 67 +3
Branches 2 2
============================================
+ Hits 60 63 +3
Misses 1 1
Partials 3 3
Continue to review full report at Codecov.
|
… the server Closes lanwen#9
42c1796
to
d5f8095
Compare
Rebase done on master, I let you decide if all is ok for you. I will try to work on #16 if possible 😉 |
great, thank you, will release that shortly |
Closes #9