Skip to content

Conversation

dwdougherty
Copy link
Collaborator

No description provided.

@dwdougherty dwdougherty requested a review from a team October 16, 2025 21:12
@dwdougherty dwdougherty self-assigned this Oct 16, 2025
Copy link
Contributor

github-actions bot commented Oct 16, 2025

DOC-5633

Copy link
Contributor

@andy-stark-redis andy-stark-redis left a comment

Choose a reason for hiding this comment

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

It's mostly really good, but I think Augie has lost the plot a little bit, here and there. He probably just needs a couple of nudges from Doggy Daddy to sort it out :-)

Comment on lines +217 to +223
}).thenAccept(res4 -> {
System.out.println(res4);
// >>> [-2]
// REMOVE_START
assertThat(res4).isEqualTo(Arrays.asList(-2L));
// REMOVE_END
}).toCompletableFuture();
Copy link
Contributor

Choose a reason for hiding this comment

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

This works fine, but the visible code reduces to thenAccept(res4 -> { System.out.println(res4);}).... This kind of lambda would normally be shortened even further into a method reference in Java 8+ style, so in the other examples, we often have a hidden thenApply(...) for the assertions, and then use the method reference .thenAccept(System.out::println) to print the output (see the suggestion for an example). It's fine if you leave the code as it is, but I was thinking maybe you could easily add a note for Augie to follow the way we use method refs in the other examples.

Suggested change
}).thenAccept(res4 -> {
System.out.println(res4);
// >>> [-2]
// REMOVE_START
assertThat(res4).isEqualTo(Arrays.asList(-2L));
// REMOVE_END
}).toCompletableFuture();
})
// REMOVE_START
.thenApply(res4 -> {
assertThat(res4).isEqualTo(Arrays.asList(-2L));
return res;
})
// REMOVE_END
.thenAccept(System.out::println)
// >>> -2
.toCompletableFuture();

// >>> Hello, World
// STEP_END
Assert.Equal("Hello, World", string.Join(", ", hValsResult));
db.KeyDelete("myhash");
Copy link
Contributor

Choose a reason for hiding this comment

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

The key deletion is in a REMOVE section in the other examples, and the assertion above probably should be visible to users either.

);

// Set expiration on hash fields using raw Execute
RedisResult hexpireRes1 = db.Execute("HEXPIRE", "myhash", 10, "FIELDS", 2, "field1", "field2");
Copy link
Contributor

Choose a reason for hiding this comment

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

This is actually implemented in the library now (HashFieldExpire(), etc) so the catchall Execute isn't necessary. I've found you sometimes need to nudge Augie in the right direction for C# because the API method names typically don't closely resemble the command names (other clients occasionally have trip-ups like this too).

Comment on lines +126 to +128
echo "\n--- HEXPIRE Command ---\n";
// Clean up first
$this->redis->del('myhash');
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't usually have these extra explanatory lines in the example output (bit of Augie randomness, I guess :-) ). Also, we'd normally have the del() command and the assertions in REMOVE sections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants