DiceDB / dice

DiceDB is a redis-compliant, reactive, scalable, highly-available, unified cache optimized for modern hardware.
https://dicedb.io/
Other
6.88k stars 1.1k forks source link

Command migration for JSON.ARRTRIM, JSON.ARRINSERT. JSON.OBJKEYS #1119

Closed ayushsatyam146 closed 1 month ago

ayushsatyam146 commented 1 month ago

Closes #1027

Command migration for JSON.ARRTRIM, JSON.ARRINSERT. JSON.OBJKEYS

soumya-codes commented 1 month ago

@ayushsatyam146 we need to migrate the integration tests to the multi-threaded RESP server.

In the PR description, I have added the checklist of tasks required to be completed for the migration of commands to multi-threaded RESP server. Please follow the same.

ayushsatyam146 commented 1 month ago

@soumya-codes I am yet to add integration tests. I raised the PR just to show activity and give updates. I'll soon add integration tests

soumya-codes commented 1 month ago

Ahh, thanks for the update. In that case can you please mark the PR status as draft?

AshwinKul28 commented 1 month ago

HI @ayushsatyam146 Thanks for the commendable efforts. Most of the things look great. I see the documents added for each commands are not exactly aligning with the sample documentation. Can you please make required changes to make it align so that we don't need to revisit this in the future. Thanks again.

ayushsatyam146 commented 1 month ago

HI @ayushsatyam146 Thanks for the commendable efforts. Most of the things look great. I see the documents added for each commands are not exactly aligning with the sample documentation. Can you please make required changes to make it align so that we don't need to revisit this in the future. Thanks again.

I'll change the docs asap

AshwinKul28 commented 1 month ago

Hey @ayushsatyam146, can you please look at the conflicts and rebase them with the master? Then, we will immediately merge the PR.

ayushsatyam146 commented 1 month ago

Hey @ayushsatyam146, can you please look at the conflicts and rebase them with the master? Then, we will immediately merge the PR.

Hi @AshwinKul28 I fixed the conflicts. Now it is good to merge. Thanks!