vietj / childprocess-vertx-ext

Child Process extension for Vert.x
Apache License 2.0
50 stars 10 forks source link

Update to Vertx 3.6.0 #3

Closed Mikey-Burns closed 5 years ago

Mikey-Burns commented 5 years ago

Update the dependencies to Vertx 3.6.0.

The other files were modified by the build. Let me know if you want me to remove them from the PR, or if they're supposed to be updated this way.

This solves #2.

Mikey-Burns commented 5 years ago

@vietj I was unable to get SpawnTest.testCat() to work locally both without my changes and with them. I'd recommend running the test yourself before this goes in to make sure I don't accidentally break the build.

Mikey-Burns commented 5 years ago

Got my workspace in order and now all of the tests are passing for me.

Mikey-Burns commented 5 years ago

@vietj Bumping this so it can get reviewed.

vietj commented 5 years ago

yes sorry for the long time :-)

vietj commented 5 years ago

I will set travis first for building PR's

vietj commented 5 years ago

actually it's there :-)

vietj commented 5 years ago

actually I'm wondering how this works, since 3.6 has a new fetch method on ReadStream and this should not compile with 3.6

Mikey-Burns commented 5 years ago

@vietj Is there something I can test regarding the ReadStream issue? It seems to be compiling locally and with Travis.

vietj commented 5 years ago

ok, I will have a look, I must be overlooking something I think

vietj commented 5 years ago

ReadStream in 3.6.0 has a fetch method and there are ReadStream impl in this project

vietj commented 5 years ago

the response is that those are not vertx stream (only for ReadStream) :-)

vietj commented 5 years ago

I think the reason is that back-pressure was bugged in the underlying project

vietj commented 5 years ago

as far as I remember

vietj commented 5 years ago

I've updated to 3.6.2 and also added support for kotlin generation