-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Implement JSON raw return types #3478
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
Conversation
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.
LGTM.
If the team agrees with the approach, I am good to go!
| CommandArgs<K, V> args = new CommandArgs<>(codec).addKey(key); | ||
|
|
||
| if (jsonPath != null) { | ||
| if (index != -1) { | ||
| // OPTIONAL as per API | ||
| args.add(jsonPath.toString()); | ||
| args.add(index); | ||
| } else if (!jsonPath.isRootPath()) { | ||
| // OPTIONAL as per API | ||
| args.add(jsonPath.toString()); | ||
| } | ||
| } |
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.
(nit) : Repeats the code in the overloaded method. We can extract it in a helper method to avoid code duplication & eventually getting methods out of sync
| CommandArgs<K, V> args = new CommandArgs<>(codec).addKey(key); | ||
|
|
||
| if (options != null) { | ||
| // OPTIONAL as per API | ||
| options.build(args); | ||
| } | ||
|
|
||
| if (jsonPaths != null) { | ||
| // OPTIONAL as per API | ||
| for (JsonPath jsonPath : jsonPaths) { | ||
| if (jsonPath != null) { | ||
| args.add(jsonPath.toString()); | ||
| } | ||
| } | ||
| } |
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.
(nit) : Repeats the code in the overloaded method. We can extract it in a helper method to avoid code duplication & eventually getting methods out of sync
This PR implements raw return types for JSON commands
See #3431