Conversation
This file contains hidden or 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
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.
Which issue does this PR close?
None.
Rationale for this change
Operatorhas async overloads that only omit the options argument.They add API surface without adding capability, and every new operation would grow the same pair.
This PR removes them and keeps one method per operation; overloads are reserved for genuinely different inputs.
The same pass simplifies
Operatorconstruction and fixes a misnamed callback alias on the Rust side.What changes are included in this PR?
Remove the
CancellationToken-only overloads ofReadAsync,WriteAsync,StatAsync, andListAsync. Tests pass the token by name instead.Rename the Rust callback alias
WriteCallbacktoVoidCallback; it reports success or failure only and is shared by every value-less operation.Move native operator construction into a private static
Constructhelper so both public constructors are a guard clause followed bySetHandle, and drop the private parameterless constructor that existed only to seed aLazy<OperatorInfo>.Infois now loaded eagerly into areadonlyfield during construction, matching the Java binding, which also removes the CA1419 suggestion on theSafeHandlesubclass.Are there any user-facing changes?
Yes. Four public overloads are removed (see below), and constructing an
Operatornow retrieves itsOperatorInfoimmediately instead of on first access toInfo, so an info retrieval failure surfaces from the constructor rather than from the property. No documentation referenced the removed overloads.Breaking changes
Affected packages: bindings/dotnet
Migration:
op.ReadAsync(path, token)withop.ReadAsync(path, cancellationToken: token).op.WriteAsync(path, bytes, token)withop.WriteAsync(path, bytes, cancellationToken: token).op.StatAsync(path, token)withop.StatAsync(path, cancellationToken: token).op.ListAsync(path, token)withop.ListAsync(path, cancellationToken: token).Calls that already pass options positionally, and calls that pass no token, are unaffected.
AI Usage Statement
claude-fable-5-1)Infodesign, and the commit layout, and reviewed every change. Assumption affecting review:Infois not read on any internal code path, so eager loading adds oneoperator_info_getcall per constructed operator, including operators created byDuplicateandWithLayer, and no per-operation cost.