Skip to content

Remove broken JSON.ARRAPPEND method from CommandObjects#3795

Closed
gerzse wants to merge 1 commit intoredis:masterfrom
gerzse:remove-unused-broken-arrappend-method
Closed

Remove broken JSON.ARRAPPEND method from CommandObjects#3795
gerzse wants to merge 1 commit intoredis:masterfrom
gerzse:remove-unused-broken-arrappend-method

Conversation

@gerzse
Copy link
Copy Markdown
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
@codecov
Copy link
Copy Markdown

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
Copy Markdown
Contributor

@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