Remove redundant map, simplify code; Show status and allow mouse click to peer to connect#21
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR removes the redundant connections map and consolidates connection tracking directly into the Peers map by embedding connection status and address fields within PeerData. The refactoring simplifies the codebase by eliminating duplicate data structures while maintaining the same functionality.
Key changes:
- Removed the
Connectionstruct andconnectionsmap from theServerstruct - Added
StatusandAddrfields toPeerDatato track connection state - Updated
ConnectToPeerto store connection status directly inPeersmap
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tsnet/tsnet.go | Removed Connection struct and connections map; moved connection tracking fields (Status, Addr) into PeerData; updated ConnectToPeer to use consolidated Peers map |
| tsnet/tsnet_test.go | Updated test to access connection status via Peers map instead of removed Connections() method |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #21 +/- ##
==========================================
- Coverage 54.59% 50.86% -3.73%
==========================================
Files 9 9
Lines 784 812 +28
==========================================
- Hits 428 413 -15
- Misses 314 357 +43
Partials 42 42 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| NotLinked ConnectionStatus = iota | ||
| Connecting | ||
| Connected | ||
| Disconnected | ||
| Failed |
There was a problem hiding this comment.
The ConnectionStatus constants lack documentation. Add a comment block explaining each state (NotLinked, Connecting, Connected, Failed) to clarify when each status is used, especially since NotLinked is now the zero value.
ldemailly
changed the title
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
tsnet/tsnet.go
Outdated
| log.S(log.Verbose, "Already known peer", log.Any("Peer", peer), log.Any("OldData", v), log.Any("NewData", data)) | ||
| // transfer the human hash (same pub key so same human hash) | ||
| data.HumanHash = v.HumanHash | ||
| // And the status |
There was a problem hiding this comment.
Corrected grammar in comment from 'And the status' to 'Transfer the status'.
| // And the status | |
| // transfer the status |
main.go
Outdated
| ap.OnMouse = func() { | ||
| if !ap.LeftClick() || !ap.MouseRelease() { | ||
| return | ||
| } | ||
| line := ap.My - 4 // account for our line and header |
There was a problem hiding this comment.
The magic number 4 used for offset calculation lacks explanation. Consider defining a named constant (e.g., headerOffset = 4) or expanding the comment to clarify what components make up this offset (e.g., 'account for status line, header line, and 2 lines of padding').
| ap.OnMouse = func() { | |
| if !ap.LeftClick() || !ap.MouseRelease() { | |
| return | |
| } | |
| line := ap.My - 4 // account for our line and header | |
| // Offset for mouse line calculation: | |
| // 1 line for status, 1 line for header, and 2 lines of padding. | |
| const headerOffset = 4 | |
| ap.OnMouse = func() { | |
| if !ap.LeftClick() || !ap.MouseRelease() { | |
| return | |
| } | |
| line := ap.My - headerOffset // account for status line, header line, and 2 lines of padding |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
No description provided.