vert-x3 / vertx-4-migration-guide

Migration to Vert.x 4 guide
https://vert-x3.github.io/vertx-4-migration-guide/index.html
20 stars 16 forks source link

Future<HttpClientResponse> response() uses in HttpClientRequest/HttpClientResponse is undocumenteted #54

Closed jcarranzan closed 3 years ago

jcarranzan commented 3 years ago

Context

Now in vertx 4.0.0 when you do some HttpClientRequest and get the result if you want to do after some operation(compose, succeed,...) with the result before sending it, you have to add the response method that returns the Future

This change is shown in vertx examples in the core/http/proxy/client.java : request.response().compose

https://github.com/vert-x3/vertx-examples/commit/1ddfd8c4e297fd473b29fb748b3b9905efc85a3e#diff-330da0139766c23d1b970aa604bae5ed935b15ce6800348c696b0a5f6744f74b

But this change is not documented in the migration guide, maybe it would be nice to have some comment on Http changes or miscellaneous changes.

jcarranzan commented 3 years ago

What do you think @vietj?

vietj commented 3 years ago

you mean not documented in the regular docs ?

On Mon, Dec 14, 2020 at 9:56 AM Jose Carranza notifications@github.com wrote:

What do you think @vietj https://github.com/vietj?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vert-x3/vertx-4-migration-guide/issues/54#issuecomment-744287293, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABXDCU4NHXQ3W3IJWSYZZTSUXHMVANCNFSM4UUWEY7Q .

jcarranzan commented 3 years ago

I meant in the migration guide. In the regular doc as far as I know neither.

vietj commented 3 years ago

alright, do you mind a contribution here ?

On Mon, Dec 14, 2020 at 3:16 PM Jose Carranza notifications@github.com wrote:

I meant in the migration guide. In the regular doc as far as I know neither.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vert-x3/vertx-4-migration-guide/issues/54#issuecomment-744469202, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABXDCVO2FQBOIQIYG5P3GTSUYM47ANCNFSM4UUWEY7Q .

jcarranzan commented 3 years ago

sure!

jcarranzan commented 3 years ago

We could add in the migration guide in the Http changes section and following the same styles of the other sections examples :

In vertx 4, with the HttpClientRequest is used the Future response implementation and we return the future of the HttpClientResponse in the result method.

The following example shows an example using HTTP client in {VertX} {v3x} releases:

HttpClient client = vertx.createHttpClient(options);

    client.request(HttpMethod.GET, 8443, "localhost", "/")
      .onSuccess(request -> {
        request.onSuccess(resp -> {

        //Handler some operation with the response
        });
      });

The following example shows an example using HTTP client with the result method in {VertX} {v4x} releases:

HttpClient client = vertx.createHttpClient(options);

    client.request(HttpMethod.GET, 8443, "localhost", "/")
      .onSuccess(request -> {
        request.response().onSuccess(resp -> {

        //Handler some operation with the response
        });
      });

what do you think @vietj ?

vietj commented 3 years ago

can you do a PR it's easier for commenting. thanks

jcarranzan commented 3 years ago

sure!

https://github.com/vert-x3/vertx-4-migration-guide/pull/57

sangeetaraghu commented 3 years ago

Hi @vietj, @jcarranzan,

We can close this issue. It is resolved. https://github.com/vert-x3/vertx-4-migration-guide/pull/57