Skip to content

refactor: move connectivity service into effect#3098

Open
jonathanlab wants to merge 2 commits into
07-02-refactor_first_effect_vertical_slicefrom
07-02-refactor_move_connectivity_service_into_effect
Open

refactor: move connectivity service into effect#3098
jonathanlab wants to merge 2 commits into
07-02-refactor_first_effect_vertical_slicefrom
07-02-refactor_move_connectivity_service_into_effect

Conversation

@jonathanlab

Copy link
Copy Markdown
Contributor

Problem

Changes

How did you test this?

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Copy link
Copy Markdown
Contributor Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@jonathanlab jonathanlab marked this pull request as ready for review July 2, 2026 15:22
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit 0d691ce.

@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Comments Outside Diff (1)

  1. packages/workspace-server/src/services/connectivity/service.test.ts, line 461-581 (link)

    P2 Missing coverage for the changes stream

    The old test suite had four dedicated tests verifying the status-change event behaviour: emits on going offline after confirmation, suppresses a single transient failure, emits on coming back online, and does not emit when status is unchanged. All four were deleted and have no equivalent for the new SubscriptionRef.changes path. The duplicate-suppression guard in setOnline and the threshold logic in runCheck are relied on by subscribers but are now exercised only indirectly. Adding it.effect cases that assert on SubscriptionRef.changes (or Stream.toArray over a bounded stream) would close this gap.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "refactor: move connectivity service into..." | Re-trigger Greptile

Comment thread packages/workspace-server/src/services/connectivity/service.ts Outdated
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
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