vert-x3 / vertx-consul-client

Vert.x Consul Client
Apache License 2.0
34 stars 23 forks source link

4.4.x tags are not related to 4.4 branch, and do not contain new code #107

Closed willmurnane closed 1 year ago

willmurnane commented 1 year ago

Version

Version 4.4.4 doesn't include the changes to the 4.4 branch. See the history here; no changes have propagated from 4.4 into released tags since 4.4.1.

Context

I am trying to use the new ACL API, and noticed that the consul client was still calling the old endpoint. This is fixed in the 4.4 branch but still has the old endpoint in 4.4.4.

Also note that the version number set in the pom in 4.4 is 4.4.1-SNAPSHOT.

Do you have a reproducer?

No

Steps to reproduce

mkdir tmp; cd tmp
curl -O https://repo1.maven.org/maven2/io/vertx/vertx-consul-client/4.4.4/vertx-consul-client-4.4.4.jar
unzip vertx-consul-client-4.4.4.jar
# The 4.4.4 release still contains the old URL for creating ACL tokens
javap -v io/vertx/ext/consul/impl/ConsulClientImpl.class | grep 'v1/acl/create'
vietj commented 1 year ago

those change have been done in master and not back ported to 4.x branch (https://github.com/vert-x3/vertx-consul-client/commits/4.x) . Feel free to open PR to backport them, they will be part then of 4.4.5

ViktoriiaLebedeva commented 1 year ago

Hi. I will transfer all the latest changes from master to 4.x. After that, it will be possible to release a version 4.4.5 with latest changes

willmurnane commented 1 year ago

Thanks, everyone, for making the relationship between the branches clear, and volunteering to implement the changes. I found one more minor change that I think should be applied, and left a comment on PR108. There's another instance of the old ACL endpoint in the current master and 4.x branches. I would be happy to make the change, or to let someone else do it. Thank you 😸

ruslansennov commented 1 year ago

Hi @willmurnane PRs are welcome :)

willmurnane commented 1 year ago

Viktoriia explained that the reason for the old endpoint is to support the old API as well, and the method is marked deprecated in the interface, so the changes made are all that's desirable to make. The changes made in #108 are thus sufficient, and I'll close this issue. Thanks for the prompt turnaround :)