Added support for georadius_ro, georadiusbymember_ro commands #1841
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sazzad16
reviewed
Aug 8, 2018
Contributor
sazzad16
left a comment
sazzad16
left a comment
There was a problem hiding this comment.
@s-sathish Thank you for your PR.
I think ReadOnly instead of _ro makes the code and usage more readable. @marcosnils WDYT?
Contributor
Agree. Using underscores in method names is not usually a Java convention. @s-sathish would you mind refactoring the |
marcosnils
suggested changes
Aug 9, 2018
Contributor
marcosnils
left a comment
marcosnils
left a comment
There was a problem hiding this comment.
Refactor the _ro in method names with Readonly
Contributor
Author
|
Hi @marcosnils @sazzad16 I've updated the code as per comments. |
Contributor
|
LGTM. I'll see why tests do not pass and approve. |
Contributor
|
@s-sathish As master branch was reset, could you please rebase your work? |
* Add SSL support to JedisCluster * Remove unused imports from SSLJedisClusterTest * Add JedisClusterPortMap interface for mapping between plaintext and SSL ports. * Fix casing in JedisClusterPortMap * Add new constructor for JedisClusterInfoCache instead of modifying existing one to fix backwards compatibility issue * Undo unintended formatting changes. * Improve name of test case in SSLJedisClusterTest * Fix logic for closing JedisCluster objects in SSLJedisClusterTest * Add option to specify mapping between redis and ssl hostnames * Remove unneeded public access modifiers
Update as per comments
Update as per comments
Contributor
Author
|
Hi @sazzad16 I just did the rebase. Please check. |
Contributor
|
hey @s-sathish seems like you're rebasing with an old version of the master branch. Can you please check as tests dont pass? |
This reverts commit 17901ba.
marcosnils
pushed a commit
that referenced
this pull request
Aug 14, 2018
* Add SSL support to JedisCluster (#1550) * Add SSL support to JedisCluster * Remove unused imports from SSLJedisClusterTest * Add JedisClusterPortMap interface for mapping between plaintext and SSL ports. * Fix casing in JedisClusterPortMap * Add new constructor for JedisClusterInfoCache instead of modifying existing one to fix backwards compatibility issue * Undo unintended formatting changes. * Improve name of test case in SSLJedisClusterTest * Fix logic for closing JedisCluster objects in SSLJedisClusterTest * Add option to specify mapping between redis and ssl hostnames * Remove unneeded public access modifiers * Support for geo RO commands. * Support for geo RO commands. * Support for geo RO commands. * Change _ro to Readonly for better understanding. * Change _ro to Readonly for better understanding Update as per comments * Change _ro to Readonly for better understanding Update as per comments * Minor update. Update commands. https://redis.io/commands/georadius * Update as per comments. * Revert "Add SSL support to JedisCluster (#1550)" This reverts commit 17901ba.
Contributor
|
Done!. Thx a lot @s-sathish for the patience and the effort. |
joyang1
pushed a commit
to joyang1/jedis
that referenced
this pull request
Dec 27, 2018
…1841) * Add SSL support to JedisCluster (redis#1550) * Add SSL support to JedisCluster * Remove unused imports from SSLJedisClusterTest * Add JedisClusterPortMap interface for mapping between plaintext and SSL ports. * Fix casing in JedisClusterPortMap * Add new constructor for JedisClusterInfoCache instead of modifying existing one to fix backwards compatibility issue * Undo unintended formatting changes. * Improve name of test case in SSLJedisClusterTest * Fix logic for closing JedisCluster objects in SSLJedisClusterTest * Add option to specify mapping between redis and ssl hostnames * Remove unneeded public access modifiers * Support for geo RO commands. * Support for geo RO commands. * Support for geo RO commands. * Change _ro to Readonly for better understanding. * Change _ro to Readonly for better understanding Update as per comments * Change _ro to Readonly for better understanding Update as per comments * Minor update. Update commands. https://redis.io/commands/georadius * Update as per comments. * Revert "Add SSL support to JedisCluster (redis#1550)" This reverts commit 17901ba.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.