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

Remove broken JSON.ARRAPPEND method from CommandObjects #3795

Closed

Conversation

gerzse
Copy link
Contributor

@gerzse gerzse commented Apr 1, 2024

One of the jsonArrAppend methods in CommandObjects has broken serialization. Instead of deserializing the response into a List, it's only expecting Long. Luckily this method is not used, so better remove it.

One of the jsonArrAppend methods in CommandObjects has broken
serialization. Instead of deserializing the response into a List<Long>,
it's only expecting Long. Luckily this method is not used, so better
remove it.
@gerzse gerzse requested a review from sazzad16 April 1, 2024 20:49
Copy link

codecov bot commented Apr 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.68%. Comparing base (fac0ae9) to head (bd05ea6).
Report is 2 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3795      +/-   ##
============================================
+ Coverage     79.61%   79.68%   +0.06%     
- Complexity     5701     5705       +4     
============================================
  Files           301      301              
  Lines         15273    15269       -4     
  Branches       1190     1189       -1     
============================================
+ Hits          12160    12167       +7     
+ Misses         2532     2525       -7     
+ Partials        581      577       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sazzad16 sazzad16 added breakingchange Pull request that has breaking changes. Must include the breaking behavior in release notes. maintenance labels Apr 3, 2024
@sazzad16 sazzad16 added this to the 5.2.0 milestone Apr 3, 2024
sazzad16
sazzad16 previously approved these changes Apr 3, 2024
Copy link
Collaborator

@sazzad16 sazzad16 left a comment

Choose a reason for hiding this comment

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

It's not broken. RedisJSON v1 returns a Long in this case.
RedisJSON v1 support is already deprecated. So deprecating this method is the safest approach IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breakingchange Pull request that has breaking changes. Must include the breaking behavior in release notes. maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants