-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add a Redis extension #9370
Add a Redis extension #9370
Conversation
5b6f88c
to
b10c8c7
Compare
Nice! I'll take a look on Monday |
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.
I added a couple initial high level comments
...ions/vertx-redis/runtime/src/main/java/io/quarkus/vertx/redis/runtime/RedisAPIsProducer.java
Outdated
Show resolved
Hide resolved
cb7a3b8
to
bffa19b
Compare
I haven't been able to look at this because of the rush for |
extensions/vertx-redis/runtime/src/main/java/io/quarkus/vertx/redis/SyncRedisAPI.java
Outdated
Show resolved
Hide resolved
...sions/vertx-redis/runtime/src/main/java/io/quarkus/vertx/redis/runtime/RedisAPIProducer.java
Outdated
Show resolved
Hide resolved
I think this is great work! |
Great thanks for the review. I'll go on and update the suggested change about |
bffa19b
to
ff70985
Compare
...sions/vertx-redis/runtime/src/main/java/io/quarkus/vertx/redis/runtime/RedisAPIProducer.java
Outdated
Show resolved
Hide resolved
extensions/vertx-redis/runtime/src/main/java/io/quarkus/vertx/redis/runtime/RedisConfig.java
Outdated
Show resolved
Hide resolved
53608c4
to
2a10b8a
Compare
2a10b8a
to
1c39633
Compare
About the naming, could you use the same convention as the other projects: in the "root" package having a |
001ceb7
to
9650b0d
Compare
9650b0d
to
4685fee
Compare
@gsmet @cescoffier ping, can you have a look? I have taken care of all the requested changes. |
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.
I added some very minor comments.
I think we should merge it once these small things sorted out and we can deal with the synthetic bean thing later if needed.
core/deployment/src/main/java/io/quarkus/deployment/Feature.java
Outdated
Show resolved
Hide resolved
...client/deployment/src/main/java/io/quarkus/redis/client/deployment/RedisClientProcessor.java
Show resolved
Hide resolved
...client/deployment/src/main/java/io/quarkus/redis/client/deployment/RedisClientProcessor.java
Outdated
Show resolved
Hide resolved
extensions/redis-client/runtime/src/main/java/io/quarkus/redis/client/runtime/RedisConfig.java
Outdated
Show resolved
Hide resolved
@machi1990 nice work. Could you have a look at the minor comments above? Once these minor things are fixed, I would merge it and then deal with the synthetic bean thing in a followup if really needed. |
4685fee
to
495e017
Compare
@gsmet thanks for the review. Comments addressed. |
Creates a redis-client extension that provides different flavours of redis clients: Imperative and Reactive client. Fixes quarkusio#4453
495e017
to
76a6fcb
Compare
Cool! Let's get this one in. Thanks @machi1990 ! |
Creates a vertx redis extension that provides different flavours of RedisAPI.
fixes #4453
QS PR here quarkusio/quarkus-quickstarts#572
/cc @pmlopes @geoand @gsmet @cescoffier