Skip to content

Improve alarm init#122

Open
syyyr wants to merge 8 commits into
mainfrom
improve-alarm-init
Open

Improve alarm init#122
syyyr wants to merge 8 commits into
mainfrom
improve-alarm-init

Conversation

@syyyr
Copy link
Copy Markdown
Collaborator

@syyyr syyyr commented May 27, 2026

No description provided.

@syyyr syyyr requested a review from j4r0u53k May 27, 2026 14:16
@syyyr syyyr force-pushed the improve-alarm-init branch 2 times, most recently from 25dc4b6 to 90739ba Compare May 27, 2026 15:14
@syyyr syyyr force-pushed the improve-alarm-init branch 4 times, most recently from 264fdaf to 17f727c Compare May 27, 2026 15:38
@syyyr syyyr force-pushed the improve-alarm-init branch from 43fcf47 to ae71a93 Compare May 27, 2026 16:26
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 27, 2026

Code coverage report for 5dc11c2

Coverage difference

❌ Code coverage worsened.

Filename Stmts Miss Cover Missing
src/sites.rs +104 +19 +0.29% 568, 569, 570, 571, 572, 573, 574, 575, 576, 578, 579, 580, 582, 583, 584, 585, 588, 589, 590, 591, 592, 593, 594, 597, 604, 606, 612, 696, 728, 805
TOTAL +104 +19 +0.47%

Overall coverage

Click to expand
Filename Stmts Miss Cover Missing
src/lib.rs 110 30 72.73% 68-89, 133, 137, 141, 176, 184, 195-199, 215-223
src/alarm.rs 236 86 63.56% 28-38, 44-46, 49-50, 56-124, 140-142, 149-151, 156, 162, 166-174, 183, 224, 254
src/cleanup.rs 63 35 44.44% 54-109
src/sync.rs 659 251 61.91% 61-62, 72, 80, 88, 97, 148, 150-151, 174, 179, 195, 238-244, 324-341, 348-350, 361-370, 447, 495-721, 757-759, 801, 834-855, 862, 897-914, 918-928, 937
src/pushlog.rs 107 107 0.00% 21-164
src/alarmlog.rs 64 64 0.00% 29-103
src/tree.rs 433 433 0.00% 78-718
src/util.rs 300 59 80.33% 32, 59-60, 63-66, 68, 83-88, 100, 140-186, 200, 217, 239, 244-252, 256, 267, 274, 281, 308, 322, 462-467
src/sites.rs 757 152 79.92% 171-203, 215-216, 237-269, 300-310, 414-415, 423-424, 429-439, 500-506, 529, 568-585, 588-597, 604-618, 668, 696, 714, 728, 736-738, 742, 746, 759-762, 771-773, 777, 792-805, 814, 820
src/dirtylog.rs 472 29 93.86% 73, 96-100, 115-119, 121, 126-127, 142, 151-153, 165-166, 172-176, 213-217, 225-226, 257, 327
src/getlog.rs 685 98 85.69% 34-148, 263, 273-279, 324, 328-332
src/bin/hp.rs 55 55 0.00% 58-127
TOTAL 3941 1399 64.50%

HTML reports

Comment thread src/sites.rs Outdated
Comment on lines +725 to +735
let mut mntchng_next = if !state.mntchng_subscribers.is_empty() {
state.mntchng_subscribers.next().left_future()
} else {
futures::future::pending().right_future()
}.fuse();

let mut subscribers_next = if !state.subscribers.is_empty() {
state.subscribers.next().left_future()
} else {
futures::future::pending().right_future()
}.fuse();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

tokio::select! and branch guards can be used instead:

                    mntchng_frame = state.mntchng_subscribers.next(), if !state.mntchng_subscribers.is_empty() => {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread src/sites.rs Outdated
Comment on lines +363 to +366
match old_subscribers.remove(&key) {
Some(subscriber) => {
subscribers.insert(key, subscriber);
None
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doing these side effects in a filter_map seems a bit odd to me.

What about?

for (path, signal) in subscriptions {
        let key = format!("{path}:*:{signal}");

        if let Some(subscriber) = old_subscribers.remove(&key) {
            // Keep existing subscriber
            subscribers.insert(key, subscriber);
        } else {
            // Queue up creation for missing subscriber
            new_sub_futures.push(async move {
                let subscriber = subscribe(client_cmd_tx, &path, &signal).await;
                (key, subscriber)
            });
        }
    }

    // Await all new subscriptions concurrently and merge them in
    for (key, subscriber) in join_all(new_sub_futures).await {
        subscribers.insert(key, subscriber);
    }

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

@syyyr syyyr force-pushed the improve-alarm-init branch from ae71a93 to 77bda3c Compare May 28, 2026 14:31
@syyyr syyyr requested a review from j4r0u53k May 28, 2026 14:31
The signal is not sent as a reponse to the Disconnected event.
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.

2 participants