Conversation
|
Appveyor doesn't care for me 😆 |
NickCraver
left a comment
NickCraver
left a comment
There was a problem hiding this comment.
Added comments on code so far (looking good, mostly formatting) - will check out tests in the AM with fresh eyes :)
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
left a comment
NickCraver
left a comment
There was a problem hiding this comment.
I think this is looking good - added a few more thoughts, mainly around:
- Using regions like a heathen!
- ...and restricting the result processor more to the specific nil case :)
| 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. |
NickCraver
left a comment
NickCraver
left a comment
There was a problem hiding this comment.
Your fantastic region-free artisanal code is looking awesome, thanks for this!
|
@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"? 😆 |
2 participants
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?