Skip to content
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

Java: README.md #388

Closed
wants to merge 17 commits into from
Closed

Conversation

cyip10
Copy link

@cyip10 cyip10 commented Jun 25, 2024

yipin-chen and others added 12 commits June 21, 2024 17:31
* Python: Added COPY command (#383)

Python: Added COPY command

* Updated CHANGELOG.md

* Addressed review comments
* Java: Add `SORT` and `SORT_RO` commands (#363)

Co-authored-by: Yury-Fridlyand <[email protected]>
* Python: add XREVRANGE command

* Update doc for xrevrange

Signed-off-by: Andrew Carbonetto <[email protected]>

* Update transaction docs

Signed-off-by: Andrew Carbonetto <[email protected]>

---------

Signed-off-by: Andrew Carbonetto <[email protected]>
Co-authored-by: Andrew Carbonetto <[email protected]>
* Java: Add `FUNCTION DUMP` and `FUNCTION RESTORE`. (#370)

* Add `FUNCTION DUMP` and `FUNCTION RESTORE` implementations.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Address PR comments.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Add tests.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Address PR comments.

Signed-off-by: Yury-Fridlyand <[email protected]>

---------

Signed-off-by: Yury-Fridlyand <[email protected]>

* Use GlideString

Signed-off-by: Andrew Carbonetto <[email protected]>

* Clean up FUNCTION DUMP & RESTORE

Signed-off-by: Andrew Carbonetto <[email protected]>

* Update handlers

Signed-off-by: Andrew Carbonetto <[email protected]>

* Update comments

Signed-off-by: Andrew Carbonetto <[email protected]>

* Add cluster IT test

Signed-off-by: Andrew Carbonetto <[email protected]>

* quick review comment

Signed-off-by: Andrew Carbonetto <[email protected]>

* SPOTLESS

Signed-off-by: Andrew Carbonetto <[email protected]>

---------

Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Andrew Carbonetto <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>
support bitcount with GlideString
support bitop, bitpos and xdel with GlideString
valkey-io#1630)

support hincrby, hincrbyfloat, incrby and incrbyfloat with GlideString
support llen, strlen, xlen and hstrlen with GlideString
valkey-io#1634)

support zintercard, zdiffstore, zremrangebyrank, lpush, lpushx, lrem, rpus and rpushx with GlideString
* Python: add XREAD command

* Minor doc fix

* PR suggestions
**Dependencies installation for Ubuntu**

Copy link

@Yury-Fridlyand Yury-Fridlyand Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A user of java wrapper don't need rust, gcc, git and other things. Only java and maybe openssl.
Protobuf is also not needed for the client user.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. Java users should only need to import the JAR library as a dependency from Maven or Gradle - or download. Building from source should be part of the DEVELOPER.md. You could reference the DEVELOPER.md if you want to point them to how to build from source.

java/README.md Outdated
OpenJDK 64-Bit Server VM 18.9 (build 11.0.1+13, mixed mode)
```bash
echo $JAVA_HOME
java -version
```

#### Building and installation steps

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in developer.md. I think you already added all what we need, so delete this section in this file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep this to make sure they are running it on jdk11

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. So keep that block (java version check) and jdk installation. All the rest installation could be removed.

java/README.md Outdated
Comment on lines 171 to 174
assert setResponse.get() == "OK" : "Failed on client.set("key", "foobar") request";

CompletableFuture<String> getResponse = client.get("key");
assert getResponse.get() == "foobar" : "Failed on client.get("key") request";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert setResponse.get() == "OK" : "Failed on client.set("key", "foobar") request";
CompletableFuture<String> getResponse = client.get("key");
assert getResponse.get() == "foobar" : "Failed on client.get("key") request";
assert setResponse.get() == "OK" : "Failed on client.set(\"key\", \"foobar\") request";
CompletableFuture<String> getResponse = client.get("key");
assert getResponse.get() == "foobar" : "Failed on client.get(\"key\") request";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use GlideString? APIs with String to be removed soon.


Maven (AARCH_64) specific.
- **IMPORTANT** must include a `classifier` block. Please use this dependency block instead and add it to the pom.xml file.
```bash

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```bash
```xml

java/README.md Outdated

```java
import glide.api.RedisClient;
import glide.api.ValkeyClient;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import glide.api.ValkeyClient;
import glide.api.GlideClient;

Update below too

- Copy the snippet and paste it in the `build.gradle` dependencies section.

Maven (AARCH_64) specific.
- **IMPORTANT** must include a `classifier` block. Please use this dependency block instead and add it to the pom.xml file.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we include classifier for mac x64 or linux x64?

Gradle:
- Copy the snippet and paste it in the `build.gradle` dependencies section.

Maven (AARCH_64) specific.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add example in gradle syntax.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something to copy-paste ;)

@cyip10 cyip10 force-pushed the java/dev_cyip10_readme branch from f9c42eb to a35414e Compare June 27, 2024 04:17
@cyip10 cyip10 mentioned this pull request Jun 27, 2024
@@ -49,6 +49,9 @@
* Python: Added ZREVRANK command ([#1614](https://github.com/aws/glide-for-redis/pull/1614))
* Python: Added XDEL command ([#1619](https://github.com/aws/glide-for-redis/pull/1619))
* Python: Added XRANGE command ([#1624](https://github.com/aws/glide-for-redis/pull/1624))
* Python: Added COPY command ([#1626](https://github.com/aws/glide-for-redis/pull/1626))
* Python: Added XREVRANGE command ([#1625](https://github.com/aws/glide-for-redis/pull/1625))
* Python: Added XREAD command ([#1644](https://github.com/aws/glide-for-redis/pull/1644))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rebase please...

**Dependencies installation for Ubuntu**

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. Java users should only need to import the JAR library as a dependency from Maven or Gradle - or download. Building from source should be part of the DEVELOPER.md. You could reference the DEVELOPER.md if you want to point them to how to build from source.

Gradle:
- Copy the snippet and paste it in the `build.gradle` dependencies section.

Maven (AARCH_64) specific.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something to copy-paste ;)

@Yury-Fridlyand
Copy link

Supereded by #395

@Yury-Fridlyand Yury-Fridlyand deleted the java/dev_cyip10_readme branch June 27, 2024 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants