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.

Sign up for GitHub

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

Jump to bottom

Java: add JSON.SET and JSON.GET #2462

Merged
merged 7 commits into from
Oct 17, 2024
Merged

Java: add JSON.SET and JSON.GET #2462

merged 7 commits into from
Oct 17, 2024

Conversation

jamesx-improving
Copy link
Collaborator

@jamesx-improving jamesx-improving commented Oct 15, 2024 •
edited
Loading

Issue link

This Pull Request is linked to issue (URL): #2430

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated.
  • CHANGELOG.md and documentation files are updated.
  • Destination branch is correct - main or release
  • Commits will be squashed upon merging.

@jamesx-improving jamesx-improving requested a review from a team as a code owner October 15, 2024 23:45
@shohamazon
Copy link
Collaborator

I have added documentation change in #2372

@jamesx-improving jamesx-improving force-pushed the java/jamesx-json-set-get branch from f9a7e68 to 0fa5309 Compare October 16, 2024 09:02
acarbonetto
acarbonetto reviewed Oct 16, 2024
View reviewed changes
java/client/src/main/java/glide/api/commands/servermodules/GlideJson.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/GlideJson.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/GlideJson.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/GlideJson.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/GlideJson.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/GlideJson.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/GlideJson.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/GlideJson.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/GlideJson.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/models/commands/json/JsonGetOptions.java Outdated Show resolved Hide resolved
@jamesx-improving jamesx-improving force-pushed the java/jamesx-json-set-get branch from a16554d to 926677e Compare October 16, 2024 17:04
Yury-Fridlyand
Yury-Fridlyand reviewed Oct 16, 2024
View reviewed changes
Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand left a comment •
edited
Loading

Choose a reason for hiding this comment

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

GlideString support is TBD

java/client/src/main/java/glide/api/commands/servermodules/GlideJson.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/GlideJson.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/models/commands/json/JsonGetOptions.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/models/commands/json/JsonGetOptions.java Show resolved Hide resolved
java/client/src/main/java/glide/api/models/commands/json/JsonGetOptions.java Show resolved Hide resolved
java/client/src/main/java/module-info.java Show resolved Hide resolved
java/integTest/src/test/java/glide/modules/JsonTests.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/GlideJson.java Outdated Show resolved Hide resolved
@jamesx-improving jamesx-improving force-pushed the java/jamesx-json-set-get branch from 926677e to 1881895 Compare October 17, 2024 10:00
@jamesx-improving jamesx-improving force-pushed the java/jamesx-json-set-get branch from ae856fc to 91573c7 Compare October 17, 2024 18:51
@Yury-Fridlyand Yury-Fridlyand added the java issues and fixes related to the java client label Oct 17, 2024
Yury-Fridlyand
Yury-Fridlyand reviewed Oct 17, 2024
View reviewed changes
java/client/src/main/java/glide/api/commands/servermodules/Json.java Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/Json.java Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/Json.java Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/Json.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/Json.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/Json.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/Json.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/commands/servermodules/Json.java Show resolved Hide resolved
java/client/src/main/java/glide/api/models/commands/json/JsonGetOptions.java Show resolved Hide resolved
java/client/src/main/java/glide/api/models/commands/json/JsonGetOptionsBinary.java Show resolved Hide resolved
acarbonetto
acarbonetto approved these changes Oct 17, 2024
View reviewed changes
Copy link
Contributor

@acarbonetto acarbonetto left a comment

Choose a reason for hiding this comment

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

Please address Yury's comments

Yury-Fridlyand
Yury-Fridlyand approved these changes Oct 17, 2024
View reviewed changes
Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

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

LGTM assuming you don't forget addressing the rest comments.

@jamesx-improving
Java: add JSON.SET and JSON.GET
4e97650 
Signed-off-by: James Xin <james.xin@improving.com>
@jamesx-improving
linter fix
afc0372 
Signed-off-by: James Xin <james.xin@improving.com>
@jamesx-improving
CHANGELOG
bccc5f1 
Signed-off-by: James Xin <james.xin@improving.com>
@jamesx-improving
remove temp gradle task
abe29de 
Signed-off-by: James Xin <james.xin@improving.com>
@jamesx-improving
address comments
7c21dc0 
Signed-off-by: James Xin <james.xin@improving.com>
@jamesx-improving
address comment 2
01f6b53 
Signed-off-by: James Xin <james.xin@improving.com>
@jamesx-improving
address comment 3
710ed04 
Signed-off-by: James Xin <james.xin@improving.com>
@jamesx-improving jamesx-improving force-pushed the java/jamesx-json-set-get branch from c2a8321 to 710ed04 Compare October 17, 2024 21:10
@acarbonetto acarbonetto merged commit adcc76f into release-1.2 Oct 17, 2024
8 checks passed
@acarbonetto acarbonetto deleted the java/jamesx-json-set-get branch October 17, 2024 21:52
@Yury-Fridlyand Yury-Fridlyand mentioned this pull request Oct 18, 2024
Closed
22 tasks
avifenesh pushed a commit to avifenesh/valkey-glide that referenced this pull request Oct 21, 2024
@jamesx-improving @avifenesh
Java: add JSON.SET and JSON.GET (valkey-io#2462)
11f88ba 
* Java: add JSON.SET and JSON.GET

---------

Signed-off-by: James Xin <james.xin@improving.com>
avifenesh pushed a commit to avifenesh/valkey-glide that referenced this pull request Oct 21, 2024
@jamesx-improving @avifenesh
Java: add JSON.SET and JSON.GET (valkey-io#2462)
6eedffc 
* Java: add JSON.SET and JSON.GET

---------

Signed-off-by: James Xin <james.xin@improving.com>
Muhammad-awawdi-amazon pushed a commit to Muhammad-awawdi-amazon/valkey-glide that referenced this pull request Oct 22, 2024
@jamesx-improving @Muhammad-awawdi-amazon
Java: add JSON.SET and JSON.GET (valkey-io#2462)
ac51bb5 
* Java: add JSON.SET and JSON.GET

---------

Signed-off-by: James Xin <james.xin@improving.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prateek-kumar-improving prateek-kumar-improving prateek-kumar-improving left review comments

@acarbonetto acarbonetto acarbonetto approved these changes

@Yury-Fridlyand Yury-Fridlyand Yury-Fridlyand approved these changes

Assignees
No one assigned
Labels
java issues and fixes related to the java client
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jamesx-improving @shohamazon @Yury-Fridlyand @acarbonetto @prateek-kumar-improving