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

Refactor the code to separate out the store package #466

Merged

Conversation

soumya-codes
Copy link
Contributor

@soumya-codes soumya-codes commented Sep 5, 2024

This PR is the first major in the many set of refactoring PRs required for moving DiceDB to multithreading.

This PR refactors the existing code by doing the following:

  1. Carve out the store code into a separate package under /internal.
  2. Move regex to an independent package under /internal
  3. Move diceerrors from /core to /internal

Note: There is lot of movement of code and takes a lot of effort to rebase and verify changes in this PR so there are couple of requests from my side:

  1. Please DO NOT merge any further PRs till this PR is merged.
  2. Please review the code changes thoroughly.

cc @JyotinderSingh @lucifercr07 @AshwinKul28 @pratikpandey21

@soumya-codes soumya-codes force-pushed the soumyap/refactor-store-code branch 2 times, most recently from 1b6c38d to d0c24e6 Compare September 5, 2024 22:23
@AshwinKul28
Copy link
Contributor

Going through it, just a general comment, can we make internal/diceerrors folder rename it to internal/errors. It doesn't make sense of having dice as a prefix to any of these internal folders. You can make the change directly by replacing it now.
@soumya-codes

@soumya-codes
Copy link
Contributor Author

soumya-codes commented Sep 5, 2024

can we make internal/diceerrors folder

Can I do that in the subsequent PRs? There are a lot of areas of improvement to be done in naming...

@soumya-codes soumya-codes changed the title Refactor the code to separate out the store layer Refactor the code to separate out the store package Sep 6, 2024
Copy link
Collaborator

@JyotinderSingh JyotinderSingh left a comment

Choose a reason for hiding this comment

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

Thanks for this massive effort @soumya-codes! The changes look really well thought out.
Have left one minor comment.

Signed-off-by: soumya-codes <151079203+soumya-codes@users.noreply.github.com>
@soumya-codes soumya-codes force-pushed the soumyap/refactor-store-code branch from d0c24e6 to fbc31c8 Compare September 6, 2024 04:00
Copy link
Collaborator

@JyotinderSingh JyotinderSingh left a comment

Choose a reason for hiding this comment

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

LGTM

@soumya-codes soumya-codes merged commit d32926b into DiceDB:master Sep 6, 2024
2 checks passed
@soumya-codes soumya-codes deleted the soumyap/refactor-store-code branch September 6, 2024 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants