cleanup: replace "OK" string returns with void by currantw · Pull Request #230 · valkey-io/valkey-glide-csharp

currantw · 2026-03-19T19:23:59Z

Summary

Several async methods returned Task<string> where the only possible success value was "OK" — the "OK" string carries no information beyond "the operation succeeded," which is already conveyed by the task completing without throwing.

This PR changes these methods to return Task (void) instead. For cluster variants, Task<ClusterValue<string>> wrapping "OK" similarly becomes Task (void).

Additionally, this PR introduces ValkeyValue.Ok (convenient representation for ValkeyValue with "OK"). Batch operation return "OK", aligning with other Valkey GLIDE clients.

Issue Link

⚪️ None

Features and Behaviour Changes

Methods that previously returned Task<string> with "OK" now return Task (void).
Cluster variants that returned Task<ClusterValue<string>> with "OK" now return Task (void).
Added ValkeyValue.Ok static property — wrapping the string "OK" (similar to existing Null and EmptyString).
Added a Ok() request builder helper in Request.cs that returns ValkeyValue.Ok.
All void-command request builders now use the Ok() helper, returning Cmd<string, ValkeyValue> instead of Cmd<string, object?> or Cmd<string, string>.
Batch result arrays now contain ValkeyValue.Ok for void commands, maintaining compatibility with other Valkey GLIDE clients.

Affected methods:

ScriptFlushAsync
ScriptKillAsync
FunctionFlushAsync
FunctionDeleteAsync
FunctionKillAsync
FunctionRestoreAsync
SelectAsync
ConfigResetStatisticsAsync
ConfigRewriteAsync
ConfigSetAsync
FlushAllDatabasesAsync
FlushDatabaseAsync
ClientSetName
HashSetAsync (HMSet overload)
HyperLogLogMergeAsync (both overloads)
KeyRestoreAsync
KeyRestoreDateTimeAsync
ListTrimAsync
ListSetByIndexAsync

Implementation

The fix is applied across three layers:

ValkeyValue.Ok constant (abstract_APITypes/ValkeyValue.cs): Added public static ValkeyValue Ok { get; } = new ValkeyValue("OK") alongside Null and EmptyString.
Cmd() helper and request builders (Internals/Request.cs, Internals/Request.*.cs): Added a Ok(RequestType, GlideString[]) helper that returns Cmd<string, ValkeyValue> with converter _ => ValkeyValue.Ok. All void-command request builders across 7 partial files now use this helper.
Command interfaces (Commands/I*.cs): Changed return types from Task<string> / Task<ClusterValue<string>> to Task. Updated XML doc comments accordingly.
Client implementations (BaseClient.*.cs, GlideClient.cs, GlideClient.ScriptingCommands.cs, GlideClusterClient.cs, GlideClusterClient.ScriptingCommands.cs): Changed return await Command(...) to await Command(...). For cluster methods, removed .ToClusterValue(route) calls.

StackExchange.Redis Compatibility:

The primary methods changed (ScriptFlushAsync, ScriptKillAsync, FunctionFlushAsync, FunctionDeleteAsync, FunctionKillAsync, FunctionRestoreAsync, SelectAsync) are not part of the StackExchange.Redis 2.8.58 API surface. The internal request builder changes actually improve SER alignment - previously HashSetAsync, HyperLogLogMergeAsync, ListSetByIndexAsync, ListTrimAsync, and KeyRestoreAsync returned Task in SER but Task<string> in Valkey GLIDE.
While ValkeyValue is an StackExchange.Redis-compatible API, this change is additive - it does not impact SER-compatibility.

Limitations

⚪️ None

Testing

Updated unit test converter assertions (CommandTests.cs) to expect ValkeyValue.Ok instead of ValkeyValue.Null or raw "OK" strings
Updated batch test assertions:
- BatchTestUtils.cs: Updated expected values for ConfigSetAsync, ConfigResetStatisticsAsync, SelectAsync, ListTrim, ListSetByIndex, HashSet
- SharedBatchTests.cs: Updated BatchDumpAndRestore assertion
Updated integration tests:
- ScriptingCommandTests.cs
- StandaloneClientTests.cs
- ClusterClientTests.cs

✅ All unit and integration tests pass

Related Issues

⚪️ None

Checklist

~~This Pull Request is related to one issue.~~
Commit message has a detailed description of what changed and why.
Tests are added or updated and all checks pass.
~~CHANGELOG.md, README.md, DEVELOPER.md, and other documentation files are updated.~~
Destination branch is correct - main or release
Create merge commit if merging release branch into main, squash otherwise.

…tring return removal - Add OkStringReturnTypeTests: reflection-based tests asserting affected methods return Task (void) - Add PreservationReturnTypeTests: reflection-based tests asserting unchanged methods retain original return types - Bug condition tests confirm defect: 14 failures show methods return Task<string> instead of Task - Preservation tests pass on unfixed code, establishing baseline to protect Signed-off-by: currantw <[email protected]>

…g, object?> - Replace all OK() helper calls with Cmd<string, object?> using _ => ValkeyValue.Null pattern - Remove unused OK() helper method from Request.cs - Affected files: Request.ScriptingCommands.cs, Request.ServerManagement.cs, Request.ConnectionManagement.cs, Request.HashCommands.cs, Request.HyperLogLogCommands.cs, Request.GenericCommands.cs - Internal cleanup: ClientSetName, HashSetAsync (HMSet), HyperLogLogMergeAsync, KeyRestoreAsync, KeyRestoreDateTimeAsync already consumed as void Signed-off-by: currantw <[email protected]>

- Update IScriptingAndFunctionBaseCommands: ScriptFlushAsync, ScriptKillAsync, FunctionFlushAsync - Update IScriptingAndFunctionStandaloneCommands: FunctionDeleteAsync, FunctionKillAsync, FunctionRestoreAsync - Update IScriptingAndFunctionClusterCommands: all cluster-routed scripting/function commands - Update IServerManagementCommands and IServerManagementClusterCommands: SelectAsync - Update XML doc comments and code examples to reflect void returns Signed-off-by: currantw <[email protected]>

…g> to Task - Update BaseClient.ScriptingCommands.cs: ScriptFlushAsync, ScriptKillAsync, FunctionFlushAsync - Update GlideClient.ScriptingCommands.cs: FunctionDeleteAsync, FunctionKillAsync, FunctionRestoreAsync - Update GlideClient.cs: SelectAsync - Update GlideClusterClient.cs: SelectAsync - Update GlideClusterClient.ScriptingCommands.cs: all cluster-routed void commands - Remove .ToClusterValue(route) calls for void cluster commands Signed-off-by: currantw <[email protected]>

- Remove Assert.Equal("OK", result) patterns from integration tests - Update batch test expectations from "OK" to null for void commands - Update CommandTests converter assertions to expect ValkeyValue.Null - Update PreservationReturnTypeTests for Cmd<string, object?> builder types Signed-off-by: currantw <[email protected]>

- Use collection expressions instead of .ToArray() (IDE0305) - Remove unnecessary using directive (IDE0005) - Replace TheoryData<MethodInfo> with inline Fact to fix xUnit1045 Signed-off-by: currantw <[email protected]>

Signed-off-by: currantw <[email protected]>

Return types are enforced at compile time by the client implementations. Reflection-based checks add no value beyond what the compiler already guarantees. Signed-off-by: currantw <[email protected]>

Copilot

Pull request overview

This PR removes non-informative "OK" success return values from several async commands by changing them from Task<string> / Task<ClusterValue<string>> to Task, and updates request builders and tests accordingly.

Changes:

Updated multiple public command APIs (standalone + cluster) to return Task instead of returning "OK".
Refactored internal request builders to drop the OK() helper and use Cmd<string, object?> converters that discard the server payload.
Updated unit/integration/batch tests and XML docs to match the new void-style semantics.

Reviewed changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
tests/Valkey.Glide.UnitTests/CommandTests.cs	Updates converter assertions for commands that previously surfaced `"OK"`.
tests/Valkey.Glide.IntegrationTests/StandaloneClientTests.cs	Removes `"OK"` assertions for `SelectAsync`.
tests/Valkey.Glide.IntegrationTests/SharedBatchTests.cs	Adjusts batch assertions for restore/void-style results.
tests/Valkey.Glide.IntegrationTests/ScriptingCommandTests.cs	Removes `"OK"` result handling for scripting/function management commands (standalone + cluster routes).
tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs	Removes `"OK"` assertion for cluster `SelectAsync`.
tests/Valkey.Glide.IntegrationTests/BatchTestUtils.cs	Updates expected batch results for `SelectAsync`.
sources/Valkey.Glide/Internals/Request.cs	Removes the unused `OK()` request helper.
sources/Valkey.Glide/Internals/Request.ServerManagement.cs	Changes `Select` request to discard `"OK"` response.
sources/Valkey.Glide/Internals/Request.ScriptingCommands.cs	Changes scripting/function management requests to discard `"OK"` response.
sources/Valkey.Glide/Internals/Request.HyperLogLogCommands.cs	Changes `PFMERGE` request to discard `"OK"` response.
sources/Valkey.Glide/Internals/Request.HashCommands.cs	Changes `HMSET` request to discard `"OK"` response.
sources/Valkey.Glide/Internals/Request.GenericCommands.cs	Changes `RESTORE` requests to discard `"OK"` response.
sources/Valkey.Glide/Internals/Request.ConnectionManagement.cs	Changes `CLIENT SETNAME` request to discard `"OK"` response.
sources/Valkey.Glide/GlideClusterClient.cs	Updates `SelectAsync` to return `Task` and discard `"OK"`.
sources/Valkey.Glide/GlideClusterClient.ScriptingCommands.cs	Updates routed cluster scripting/function management APIs to return `Task`.
sources/Valkey.Glide/GlideClient.cs	Updates `SelectAsync` to return `Task` and discard `"OK"`.
sources/Valkey.Glide/GlideClient.ScriptingCommands.cs	Updates standalone function management APIs to return `Task`.
sources/Valkey.Glide/BaseClient.ScriptingCommands.cs	Updates base scripting/function management APIs to return `Task`.
sources/Valkey.Glide/Commands/ITransactionCommands.cs	Doc/example consistency updates for task-returning APIs.
sources/Valkey.Glide/Commands/ITransactionClusterCommands.cs	Doc/example consistency updates for task-returning APIs.
sources/Valkey.Glide/Commands/ITransactionBaseCommands.cs	Doc formatting/returns tag update for `WatchAsync`.
sources/Valkey.Glide/Commands/IServerManagementCommands.cs	Changes `SelectAsync` to `Task` and updates docs.
sources/Valkey.Glide/Commands/IServerManagementClusterCommands.cs	Changes `SelectAsync` to `Task` and updates docs.
sources/Valkey.Glide/Commands/IScriptingAndFunctionStandaloneCommands.cs	Changes function management return types to `Task` and updates docs/examples.
sources/Valkey.Glide/Commands/IScriptingAndFunctionClusterCommands.cs	Changes routed scripting/function management return types to `Task` and updates docs/examples.
sources/Valkey.Glide/Commands/IScriptingAndFunctionBaseCommands.cs	Changes scripting/function management return types to `Task` and updates docs/examples.
sources/Valkey.Glide/Commands/IPubSubCommands.cs	Standardizes `<returns>` docs for task-returning APIs.
sources/Valkey.Glide/Commands/IPubSubClusterCommands.cs	Standardizes `<returns>` docs for task-returning APIs.
sources/Valkey.Glide/Commands/IListCommands.cs	Standardizes `<returns>` docs for task-returning APIs.
sources/Valkey.Glide/Commands/IHyperLogLogCommands.cs	Standardizes `<returns>` docs for task-returning APIs.
sources/Valkey.Glide/Commands/IHashCommands.cs	Standardizes `<returns>` docs and minor whitespace normalization.
sources/Valkey.Glide/Commands/IGenericBaseCommands.cs	Adds/standardizes `<returns>` docs for restore APIs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sources/Valkey.Glide/GlideClusterClient.ScriptingCommands.cs

tests/Valkey.Glide.IntegrationTests/SharedBatchTests.cs

sources/Valkey.Glide/GlideClusterClient.ScriptingCommands.cs

Returning Task is the async equivalent of void — documenting <returns> on these methods adds no information. Signed-off-by: currantw <[email protected]>

sources/Valkey.Glide/Commands/IHyperLogLogCommands.cs

…sterCommands Signed-off-by: currantw <[email protected]>

Signed-off-by: currantw <[email protected]>

…o `error` now that all violations have been removed. Signed-off-by: currantw <[email protected]>

Signed-off-by: currantw <[email protected]>

Add a well-known ValkeyValue.Ok constant wrapping the string "OK", alongside Null and EmptyString, for batch result consistency. Signed-off-by: currantw <[email protected]>

Add a Void() helper in Request.cs that returns ValkeyValue.Ok for void commands, replacing the repeated Cmd<string, object?> pattern. Signed-off-by: currantw <[email protected]>

…() helper Replace all Cmd<string, object?> and Cmd<string, string> patterns for void commands with the Void() helper that returns ValkeyValue.Ok. Signed-off-by: currantw <[email protected]>

…ValkeyValue.Ok Update unit test assertions for void-command converters to expect ValkeyValue.Ok instead of ValkeyValue.Null or raw string "OK". Signed-off-by: currantw <[email protected]>

…lkeyValue.Ok Update expected values for void-command batch results in BatchTestUtils.cs and SharedBatchTests.cs. Signed-off-by: currantw <[email protected]>

Signed-off-by: currantw <[email protected]>

sources/Valkey.Glide/abstract_APITypes/ValkeyValue.cs

Signed-off-by: currantw <[email protected]>

… error to suggestion Signed-off-by: currantw <[email protected]>

Revert changes to .editorconfig files and ChannelMessageQueue.cs/ValkeyKey.cs that were not intended as part of this PR. Signed-off-by: currantw <[email protected]>

tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs

.editorconfig

sources/Valkey.Glide/Commands/IScriptingAndFunctionClusterCommands.cs

sources/Valkey.Glide/GlideClient.cs

… into currantw/remove-ok-string-returns

currantw added 8 commits March 19, 2026 11:22

style(test): fix lint errors in property test files

9079e76

- Use collection expressions instead of .ToArray() (IDE0305) - Remove unnecessary using directive (IDE0005) - Replace TheoryData<MethodInfo> with inline Fact to fix xUnit1045 Signed-off-by: currantw <[email protected]>

docs(api): standardize returns tags on void Task methods

bbd5d8f

Signed-off-by: currantw <[email protected]>

test(api): remove redundant reflection-based return type tests

a05c8fb

Return types are enforced at compile time by the client implementations. Reflection-based checks add no value beyond what the compiler already guarantees. Signed-off-by: currantw <[email protected]>

currantw requested a review from Copilot March 19, 2026 19:23

currantw self-assigned this Mar 19, 2026

Copilot started reviewing on behalf of currantw March 19, 2026 19:24 View session

Copilot AI reviewed Mar 19, 2026

View reviewed changes

docs(api): remove redundant returns tags from void Task methods

6b14863

Returning Task is the async equivalent of void — documenting <returns> on these methods adds no information. Signed-off-by: currantw <[email protected]>

currantw commented Mar 19, 2026

View reviewed changes

sources/Valkey.Glide/Commands/IHyperLogLogCommands.cs Show resolved Hide resolved

currantw added 11 commits March 19, 2026 13:02

docs(api): fix malformed XML doc comments in IScriptingAndFunctionClu…

ea685ef

…sterCommands Signed-off-by: currantw <[email protected]>

Some minor documentation cleanup.

f081ddb

Signed-off-by: currantw <[email protected]>

Fix "expression value is never used" warnings.

e83821c

Signed-off-by: currantw <[email protected]>

Upgrade csharp_style_unused_value_expression_statement_preference t…

d32f768

…o `error` now that all violations have been removed. Signed-off-by: currantw <[email protected]>

Fix failed batch tests

1bfb8c3

Signed-off-by: currantw <[email protected]>

feat(api): add ValkeyValue.Ok static property

18a8154

Add a well-known ValkeyValue.Ok constant wrapping the string "OK", alongside Null and EmptyString, for batch result consistency. Signed-off-by: currantw <[email protected]>

feat(internals): add Void request builder helper

c19fa8d

Add a Void() helper in Request.cs that returns ValkeyValue.Ok for void commands, replacing the repeated Cmd<string, object?> pattern. Signed-off-by: currantw <[email protected]>

refactor(internals): update void-command request builders to use Void…

194f37d

…() helper Replace all Cmd<string, object?> and Cmd<string, string> patterns for void commands with the Void() helper that returns ValkeyValue.Ok. Signed-off-by: currantw <[email protected]>

test(commands): update converter assertions from ValkeyValue.Null to …

6495856

…ValkeyValue.Ok Update unit test assertions for void-command converters to expect ValkeyValue.Ok instead of ValkeyValue.Null or raw string "OK". Signed-off-by: currantw <[email protected]>

test(batch): update batch test assertions from ValkeyValue.Null to Va…

72620e5

…lkeyValue.Ok Update expected values for void-command batch results in BatchTestUtils.cs and SharedBatchTests.cs. Signed-off-by: currantw <[email protected]>

Minor doc update to and rename Void to Ok.

0706d41

Signed-off-by: currantw <[email protected]>

currantw commented Mar 19, 2026

View reviewed changes

sources/Valkey.Glide/abstract_APITypes/ValkeyValue.cs Show resolved Hide resolved

currantw added 4 commits March 19, 2026 15:32

Update TestSelect to match standalone.

4b38edb

Signed-off-by: currantw <[email protected]>

Fix a few "Remove unnecessary expression value (IDE0058)" violations

06f16ca

Signed-off-by: currantw <[email protected]>

Change csharp_style_unused_value_expression_statement_preference from…

868f90d

… error to suggestion Signed-off-by: currantw <[email protected]>

revert: restore editorconfig and abstract_APITypes files to main

3ae00f3

Revert changes to .editorconfig files and ChannelMessageQueue.cs/ValkeyKey.cs that were not intended as part of this PR. Signed-off-by: currantw <[email protected]>

currantw commented Mar 19, 2026

View reviewed changes

tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs Show resolved Hide resolved

currantw commented Mar 19, 2026

View reviewed changes

.editorconfig Show resolved Hide resolved

currantw requested review from alexr-bq and xShinnRyuu March 19, 2026 22:43

xShinnRyuu reviewed Mar 19, 2026

View reviewed changes

sources/Valkey.Glide/Commands/IScriptingAndFunctionClusterCommands.cs Show resolved Hide resolved

sources/Valkey.Glide/GlideClient.cs Show resolved Hide resolved

Merge branch 'main' of https://github.com/valkey-io/valkey-glide-csharp…

88ac755

… into currantw/remove-ok-string-returns

xShinnRyuu approved these changes Mar 20, 2026

View reviewed changes

jeremyprime approved these changes Mar 20, 2026

View reviewed changes

currantw merged commit 06c12e8 into valkey-io:main Mar 20, 2026
35 of 38 checks passed

currantw deleted the currantw/remove-ok-string-returns branch March 20, 2026 21:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cleanup: replace "OK" string returns with void#230

cleanup: replace "OK" string returns with void#230
currantw merged 25 commits intovalkey-io:mainfrom
currantw:currantw/remove-ok-string-returns

currantw commented Mar 19, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

currantw commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Issue Link

Features and Behaviour Changes

Implementation

Limitations

Testing

Related Issues

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

currantw commented Mar 19, 2026 •

edited

Loading