Skip to content

Conversation

@jaironalves
Copy link

@jaironalves jaironalves commented Nov 27, 2025

Microsoft Reviewers: Open in CodeFlow

jaironalves and others added 18 commits November 21, 2025 08:57
- Create RedisJobShardManager.Log.cs with partial methods using LoggerMessage attributes
- Update RedisJobShardManager.cs to use the new partial log methods
- Follow the same pattern used in AzureStorageJobShardManager and RedisJobShard

Co-authored-by: jaironalves <[email protected]>
@jaironalves

This comment was marked as resolved.

Copilot AI and others added 10 commits December 3, 2025 22:58
- Fix TaskCompletionSource type inconsistency (use non-generic version)
- Use consistent ConfigureAwait pattern with Azure implementation
- Fix batch collection logic to use TryPeek instead of TryRead/TryWrite
- Initialize MetadataVersion from metadata dictionary in constructor
- Add local function for batch collection consistency with Azure

Co-authored-by: jaironalves <[email protected]>
Critical fix: Use expectedVersion instead of MetadataVersion when calculating newVersion in ExecuteUpdateMetadataAsync to prevent race conditions and ensure proper CAS semantics.

Add comprehensive review document detailing all findings.

Co-authored-by: jaironalves <[email protected]>
The review findings have been documented in the PR description instead.

Co-authored-by: jaironalves <[email protected]>
@jaironalves jaironalves marked this pull request as ready for review December 6, 2025 17:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

- Add braces on new lines for foreach loops in RedisJobShard.cs
- Add braces on new line for catch block in RedisJobShard.cs
- Addresses unresolved PR review comments about code formatting

Co-authored-by: jaironalves <[email protected]>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

/// <summary>
/// Represents an operation to be performed on a durable job.
/// </summary>
internal readonly struct JobOperation
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The JobOperation struct should be marked with readonly keyword for consistency with C# 13 best practices and to prevent accidental mutations. The Azure implementation at src/Azure/Orleans.DurableJobs.AzureStorage/JobOperation.cs uses struct without readonly, but given that all properties are init-only and the coding guidelines emphasize using latest C# features, this should be readonly struct.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant