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

When a prefix is dropped, the !badger!move prefixes associated with t… #1331

Merged

Conversation

sana-jawad
Copy link
Contributor

@sana-jawad sana-jawad commented May 15, 2020

…he prefix should also be removed.
Please refer #1288 for details


This change is Reviewable

@CLAassistant
Copy link

CLAassistant commented May 15, 2020

CLA assistant check
All committers have signed the CLA.

@sana-jawad sana-jawad force-pushed the BadgerMoveKeysInDropPrefix branch from c8b59ab to 272b73d Compare May 15, 2020 21:24
Copy link
Contributor

@jarifibrahim jarifibrahim left a comment

Choose a reason for hiding this comment

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

:lgtm: I'll get this approved by @manishrjain

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @ashish-goswami, @jarifibrahim, and @manishrjain)

Copy link
Contributor

@jarifibrahim jarifibrahim left a comment

Choose a reason for hiding this comment

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

:lgtm: Thank you for fixing this @sana-jawad 🎉

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @ashish-goswami, @jarifibrahim, and @manishrjain)

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm: Nice fix. Thanks, @sana-jawad .

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @ashish-goswami and @manishrjain)

@sana-jawad
Copy link
Contributor Author

sana-jawad commented May 21, 2020

:lgtm: Thank you for fixing this @sana-jawad 🎉

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @ashish-goswami, @jarifibrahim, and @manishrjain)

Thanks for the review. Can you please help me merge this pull request. I am not seeing an option to merge.

@jarifibrahim
Copy link
Contributor

jarifibrahim commented May 22, 2020

Hey @sana-jawad, I've made some minor changes to your code based on feedback. I removed the comment about move keys because 3e1cdd9 has fixed that.

I'll merge the PR once the build succeeds 🎉

@jarifibrahim jarifibrahim merged commit 1056675 into hypermodeinc:master May 22, 2020
@jarifibrahim
Copy link
Contributor

@sana-jawad I've merged your PR. Thank you for fixing this 🎉

NamanJain8 pushed a commit that referenced this pull request Sep 8, 2020
When a prefix `P` is dropped, we do not drop the `!badger!move!P` keys.
This PR fixes this issue by dropping `!badger!move!P` keys when keys
with prefix `P` are dropped via `dropPrefix()`.

(cherry picked from commit 1056675)
NamanJain8 added a commit that referenced this pull request Sep 9, 2020
When a prefix `P` is dropped, we do not drop the `!badger!move!P` keys.
This PR fixes this issue by dropping `!badger!move!P` keys when keys
with prefix `P` are dropped via `dropPrefix()`.

(cherry picked from commit 1056675)

Co-authored-by: sana-jawad <[email protected]>
jarifibrahim pushed a commit that referenced this pull request Oct 2, 2020
When a prefix `P` is dropped, we do not drop the `!badger!move!P` keys.
This PR fixes this issue by dropping `!badger!move!P` keys when keys
with prefix `P` are dropped via `dropPrefix()`.
mYmNeo pushed a commit to mYmNeo/badger that referenced this pull request Jan 16, 2023
…permodeinc#1503)

When a prefix `P` is dropped, we do not drop the `!badger!move!P` keys.
This PR fixes this issue by dropping `!badger!move!P` keys when keys
with prefix `P` are dropped via `dropPrefix()`.

(cherry picked from commit 1056675)

Co-authored-by: sana-jawad <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants