Support LMOVE by Avital-Fine · Pull Request #2065 · StackExchange/StackExchange.Redis

Choose a reason for hiding this comment

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

Thanks for this! I left comments inline - note that #2041 is a major change we wanted to get in before tons of PRs here (per #2055) and will make merging a bit difficult. I wanted to throw a note in here in case unaware. I'll do me best to help merge main into these after NRTs lands.

Co-authored-by: Nick Craver <nrcraver@gmail.com>

Co-authored-by: Nick Craver <nrcraver@gmail.com>

@Avital-Fine for tests, check out the format Skip.IfMissingFeature(conn, nameof(RedisFeatures.PushMultiple), f => f.PushMultiple); - we need to so similar for LMOVE there for testing against older servers :)

Co-authored-by: Nick Craver <nrcraver@gmail.com>

Thanks for this! I left comments inline - note that #2041 is a major change we wanted to get in before tons of PRs here (per #2055) and will make merging a bit difficult. I wanted to throw a note in here in case unaware. I'll do me best to help merge main into these after NRTs lands.

Thank you! hope the merge won't be too bad 😅

Choose a reason for hiding this comment

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

A few tweaks/removals then I think we're good to go - thanks for this!


		public RedisValue ListMove(RedisKey sourceKey, RedisKey destinationKey, ListSide whereFrom, ListSide whereTo, CommandFlags flags = CommandFlags.None)
		{
		var msg = Message.Create(Database, flags, RedisCommand.LMOVE, sourceKey, destinationKey, whereFrom == ListSide.Left ? "Left" : "right", whereTo == ListSide.Left ? "Left" : "right");

Choose a reason for hiding this comment

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

We should be consistent in Left vs right here :)

		Skip.IfMissingFeature(conn, nameof(RedisFeatures.PushMultiple), f => f.PushMultiple);
		var db = conn.GetDatabase();
		RedisKey key = "testlist";
		RedisKey key = Me();

Choose a reason for hiding this comment

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

Thanks for these fixes btw!

Choose a reason for hiding this comment

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

Sure!

Choose a reason for hiding this comment

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

LGTM 👍

Choose a reason for hiding this comment

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

👍 Looking good