eclipse / microprofile-rest-client

MicroProfile Rest Client
Apache License 2.0
141 stars 71 forks source link

[334] - add String overloaded RestClientBuilder.baseUri() method #372

Closed WhiteCat22 closed 3 months ago

WhiteCat22 commented 3 months ago

Added a String overloaded RestClientBuilder.baseUri() method to the API.

jim-krueger commented 3 months ago

@Emily-Jiang Adam is already considering TCK for this and talking with James and me about it in slack.

It will probably be best going forward to not consider that a PR is complete and ready to merge until we directly request that you merge it. Thanks

WhiteCat22 commented 3 months ago

I've finally found time to talk to both Jim and James and the consensus is that a TCK test isn't really appropriate, since the implementation is in the API. A TCK test would just be testing the JDK's implementation on URI.create() rather than the MpRestClient implementation.

Additionally, I don't think a spec change is appropriate either since we don't go into specific details for other API methods. We aren't adding new functionality that needs to be documented. We're just adding additional options for existing functionality to the API which is clearly detailed by the javadoc.

jamezp commented 3 months ago

Agreed, I don't see a need for a TCK test for a single method.

jim-krueger commented 3 months ago

@Emily-Jiang This is ready to be merged.

WhiteCat22 commented 3 months ago

resolves #334