Skip to content

Renaming work sequence to work id for dependency tracking#7250

Merged
lijunwangs merged 1 commit intoanza-xyz:masterfrom
lijunwangs:rename_work_sequence_to_id
Aug 27, 2025
Merged

Renaming work sequence to work id for dependency tracking#7250
lijunwangs merged 1 commit intoanza-xyz:masterfrom
lijunwangs:rename_work_sequence_to_id

Conversation

@lijunwangs
Copy link
Copy Markdown

Problem

Summary of Changes

Address a follow-up comment on work sequence, rename it to work id

Fixes #

@mergify
Copy link
Copy Markdown

mergify Bot commented Jul 30, 2025

If this PR represents a change to the public RPC API:

  1. Make sure it includes a complementary update to rpc-client/ (example)
  2. Open a follow-up PR to update the JavaScript client @solana/kit (example)

Thank you for keeping the RPC clients in sync with the server API @lijunwangs.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 30, 2025

Codecov Report

❌ Patch coverage is 98.27586% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.8%. Comparing base (9869ae5) to head (24ba9e6).
⚠️ Report is 2735 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #7250     +/-   ##
=========================================
- Coverage    82.8%    82.8%   -0.1%     
=========================================
  Files         803      803             
  Lines      364392   364392             
=========================================
- Hits       302035   301995     -40     
- Misses      62357    62397     +40     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lijunwangs lijunwangs requested a review from alannza July 30, 2025 23:48
Copy link
Copy Markdown

@alannza alannza left a comment

Choose a reason for hiding this comment

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

Thanks for tweaking this. Left a minor comment inline.

Comment on lines +27 to +30
/// Notify all waiting threads that a work has been processed with the given work id.
/// This function will update the processed work id and notify all waiting threads only if the work
/// id is greater than the procsessed work id. Notify a work of id number 's' will
/// implicitly imply that all work with id number less than 's' have been processed.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This comment didn't survive the rename, it doesn't make much sense anymore. How about something like this?:

Suggested change
/// Notify all waiting threads that a work has been processed with the given work id.
/// This function will update the processed work id and notify all waiting threads only if the work
/// id is greater than the procsessed work id. Notify a work of id number 's' will
/// implicitly imply that all work with id number less than 's' have been processed.
/// Notify all waiting threads that work has been processed with the given work id.
/// This function will notify all waiting threads only if they don't have unfinished work as a dependency.

@lijunwangs lijunwangs merged commit 3969b79 into anza-xyz:master Aug 27, 2025
41 checks passed
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.

3 participants