Skip to content

Fix comments of _quicklistSplitNode function.#4341

Merged
oranagra merged 3 commits intoredis:unstablefrom
tianhe1986:develop_quicklist_comment
Sep 9, 2020
Merged

Fix comments of _quicklistSplitNode function.#4341
oranagra merged 3 commits intoredis:unstablefrom
tianhe1986:develop_quicklist_comment

Conversation

@tianhe1986
Copy link
Contributor

According to the code, when 'after'==0, the returned node will have elements [0, OFFSET-1], and the input node keeps elements [OFFSET, END].

oranagra
oranagra previously approved these changes Sep 5, 2020
@oranagra oranagra added the state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten label Sep 5, 2020
@oranagra
Copy link
Member

oranagra commented Sep 5, 2020

@tianhe1986 good catch.. this is quite hard to review since the code deals with what to delete rather than what to keep.
also, being coded this way it is quite inefficient (copying a lot of date just to delete it a moment later), but i guess doing it efficiently is complicated since ziplist doesn't support such a thing.

one thing to ask before i merge this, maybe you understand why everything in the comment is written twice?
maybe the second one needs to have some "in other words:" prefix so it'll be clear that it just states the same thing and not adding further complications?

@oranagra oranagra merged commit 63730d9 into redis:unstable Sep 9, 2020
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Nov 4, 2020
Comments about the behavior of the function where wrong (off by one)
Co-authored-by: Oran Agra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants