Skip to content

rts: clarify connect method - #4060

Open
marksvc wants to merge 1 commit into
masterfrom
task/rts-connect
Open

rts: clarify connect method#4060
marksvc wants to merge 1 commit into
masterfrom
task/rts-connect

Conversation

@marksvc

@marksvc marksvc commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

RealtimeServer.connect is overloaded and used for different things.

This patch changes the overloads to add clarity to how connect is
used, not used, and the implications.


  • I want to modify the connect method in a followup change. This patch makes our connect overloads more plain, and the followup change should be less confusing as a result.
  • This patch should not be changing behaviour, but just adding clarity. (Except there is an added Warning.)
  • You may notice unpleasant things as you review this. The patch here isn't trying to improve the behaviour, but preserve it.

Open in Devin Review


This change is Reviewable

RealtimeServer.connect is overloaded and used for different things.

This patch changes the overloads to add clarity to how connect is
used, not used, and the implications.
@marksvc
marksvc marked this pull request as draft August 19, 2026 22:34
@marksvc
marksvc deployed to screenshot_diff August 19, 2026 22:42 — with GitHub Actions Active
@marksvc marksvc added testing not required e2e Run e2e tests for this pull request labels Aug 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📸 Screenshot diff deployed! (10 changes)

View the visual diff at: https://pr-4060--sf-screenshot-diffs.netlify.app

@marksvc
marksvc marked this pull request as ready for review August 19, 2026 22:43
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.10%. Comparing base (c4e4f30) to head (f6c872f).
⚠️ Report is 7 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/RealtimeServer/common/realtime-server.ts 83.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4060      +/-   ##
==========================================
+ Coverage   81.08%   81.10%   +0.01%     
==========================================
  Files         661      661              
  Lines       43061    43061              
  Branches     7062     7054       -8     
==========================================
+ Hits        34917    34925       +8     
+ Misses       6972     6963       -9     
- Partials     1172     1173       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

Labels

e2e Run e2e tests for this pull request testing not required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant