LPOS feature #2080
Merged
LPOS feature #2080
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Appveyor doesn't care for me 😆 |
NickCraver
reviewed
Apr 11, 2022
NickCraver
requested changes
Apr 12, 2022
Co-authored-by: Nick Craver <nrcraver@gmail.com>
|
@NickCraver - FYI I needed to add another ResultProcessor for handling a default value type since the Also, this line is redundant, maybe it should be nixed? |
NickCraver
requested changes
Apr 12, 2022
| public Task<long> ListPositionAsync(RedisKey key, RedisValue element, long rank = 1, long maxLength = 0, CommandFlags flags = CommandFlags.None) | ||
| { | ||
| var msg = CreateListPositionMessage(Database, flags, key, element, rank, maxLength); | ||
| return ExecuteAsync(msg, ResultProcessor.Int64DefaultNegativeOne); |
|
@slorello89 Yep I gotcha - added thoughts on there, and yeah we can nuke that line - feel free to throw in there since it'll be gone from history and is discussed here. |
slorello89
commented
Apr 12, 2022
NickCraver
approved these changes
Apr 12, 2022
|
@slorello89 Just adding release notes - will merge once builds are good :) |
|
Isn't a lack of preprocessor directives one of the pillars of "clean code"? 😆 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements the LPOS command for #2055
Had to add a new Message type to accommodate.
@NickCraver, one thing that might be slightly controversial, I set
rankto 1 andmaxlento 0 by default, this is operationally equivalent to not adding them at all but very much simplifies the message creation without having to do any other array/list allocations. LMK what you think of this.Also is there a better way to extract an array of expected Value-types? maybe a new ResultsProcessor?