improve documentation structure and simplify TCP examples#96
improve documentation structure and simplify TCP examples#96AlexStocks merged 5 commits intoAlexStocks:masterfrom
Conversation
|
Warning Rate limit exceeded@ayamzh has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 15 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughReplaces callback-focused sections in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Application
participant S as Session
participant PH as PkgHandler
participant EL as EventListener
participant Conn as Connection
participant OS as OS_Socket
rect rgba(230,245,255,0.6)
Note over Dev,S: Send path (no internal retry)
Dev->>S: WritePkg/WriteBytes(...)
S->>Conn: enqueue/send
Conn->>OS: send()
alt send ok
OS-->>Conn: bytes accepted
Conn-->>S: notify success
S-->>Dev: return nil
else disconnect or send-buffer full
OS-->>Conn: error (e.g., EPIPE/EAGAIN)
Conn-->>S: error
Note over Dev: Upper layer decides retry/backoff
S-->>Dev: return error
end
end
rect rgba(240,255,240,0.6)
Note over Conn,PH: Receive path (read goroutine -> process goroutine)
OS-->>Conn: bytes received
Conn->>S: deliver frame
S->>PH: OnMessage (decode/pack)
PH->>EL: OnMessage (app handling)
end
sequenceDiagram
autonumber
participant T as Cron/Timer
participant S as Session
participant EL as EventListener
participant Conn as Connection
rect rgba(255,248,230,0.6)
Note over T,S: Heartbeat / active-time check
T->>S: OnCron()
alt inactive beyond timeout
S->>EL: OnClose (reason: timeout)
S->>Conn: Close()
else active
S-->>T: no-op
end
end
Note over S: Active time updated on successful IO
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
README_CN.md (4)
362-368: Package alias inconsistency (transport vs getty).The quick-start imports github.com/AlexStocks/getty/transport and uses transport., but later examples use getty.. Unify to transport.* for consistency.
Apply these diffs:
-server := getty.NewTCPServer( - getty.WithLocalAddress(":8080"), // 监听地址 - getty.WithServerTaskPool(taskPool), // 任务池 +server := transport.NewTCPServer( + transport.WithLocalAddress(":8080"), // 监听地址 + transport.WithServerTaskPool(taskPool), // 任务池 )-server := getty.NewTCPServer( - getty.WithLocalAddress(":8080"), // 监听地址 - getty.WithServerTaskPool(taskPool), // 任务池 +server := transport.NewTCPServer( + transport.WithLocalAddress(":8080"), // 监听地址 + transport.WithServerTaskPool(taskPool), // 任务池 )-server := transport.NewTCPServer( - transport.WithLocalAddress(":8080"), // 监听地址 - transport.WithServerTaskPool(taskPool), // 任务池 -) +server := transport.NewTCPServer( + transport.WithLocalAddress(":8080"), // 监听地址 + transport.WithServerTaskPool(taskPool), // 任务池 +)Also applies to: 465-473, 212-221
35-53: Add language to fenced ASCII diagrams (markdownlint MD040).Specify a language for fenced blocks; use text.
-``` +```text ... -``` +```Do this for both architecture and data-flow diagrams.
Also applies to: 67-78
65-66: Fix heading levels and emphasis-as-heading (MD001/MD036).Use proper headings and correct level increments.
Apply:
-#### 完整数据流图 +### 完整数据流图-**处理顺序:** +### 处理顺序-**关键组件:** +### 关键组件Repeat similarly for “关键要点” and other bolded pseudo-headings.
Also applies to: 80-89
176-183: Guard the type assertion in example to avoid panic.Use a checked assertion to keep the sample resilient.
- messageData := pkg.([]byte) + messageData, ok := pkg.([]byte) + if !ok { + log.Printf("invalid packet type: %T", pkg) + return + }README.md (4)
362-368: Consistent package aliasing (transport vs getty).Align examples to use transport.* to match earlier imports.
-server := getty.NewTCPServer( - getty.WithLocalAddress(":8080"), // Listen address - getty.WithServerTaskPool(taskPool), // Task pool +server := transport.NewTCPServer( + transport.WithLocalAddress(":8080"), // Listen address + transport.WithServerTaskPool(taskPool), // Task pool )-server := getty.NewTCPServer( - getty.WithLocalAddress(":8080"), // Listen address - getty.WithServerTaskPool(taskPool), // Task pool +server := transport.NewTCPServer( + transport.WithLocalAddress(":8080"), // Listen address + transport.WithServerTaskPool(taskPool), // Task pool )-server := transport.NewTCPServer( - transport.WithLocalAddress(":8080"), // Listen address - transport.WithServerTaskPool(taskPool), // Task pool -) +server := transport.NewTCPServer( + transport.WithLocalAddress(":8080"), // Listen address + transport.WithServerTaskPool(taskPool), // Task pool +)Also applies to: 465-473, 212-221
35-53: Add language to fenced ASCII diagrams (markdownlint MD040).Mark these code fences as text.
-``` +```text ... -``` +```Do for both architecture and data-flow diagrams.
Also applies to: 67-78
65-66: Fix heading level increment and emphasis-as-heading (MD001/MD036).Use proper headings.
-#### Complete Data Flow Diagram +### Complete Data Flow Diagram-**Processing Order:** +### Processing Order-**Key Components:** +### Key ComponentsRepeat similarly for “Key Points”.
Also applies to: 80-89
176-183: Avoid panics in example by checking type assertion.Small tweak for robustness.
- messageData := pkg.([]byte) + messageData, ok := pkg.([]byte) + if !ok { + log.Printf("invalid packet type: %T", pkg) + return + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)README_CN.md(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-18T08:16:03.121Z
Learnt from: ayamzh
PR: AlexStocks/getty#91
File: README_CN.md:0-0
Timestamp: 2025-09-18T08:16:03.121Z
Learning: In Getty's callback system, while callbacks.Invoke() propagates panics, the session layer in transport/session.go wraps the callback invocation in a goroutine with defer/recover to handle panics safely and log them with stack traces, making the "Panic 安全性" documentation in README accurate.
Applied to files:
README.mdREADME_CN.md
🪛 markdownlint-cli2 (0.18.1)
README.md
35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
65-65: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
67-67: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
262-262: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
268-268: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
274-274: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
280-280: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
285-285: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
290-290: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
295-295: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
320-320: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
339-339: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
427-427: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
431-431: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
437-437: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
443-443: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
448-448: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
452-452: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
459-459: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
README_CN.md
35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
65-65: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
67-67: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
262-262: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
268-268: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
274-274: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
280-280: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
285-285: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
290-290: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
295-295: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
320-320: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
339-339: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
427-427: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
431-431: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
437-437: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
443-443: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
448-448: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
452-452: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
459-459: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (3)
README_CN.md (1)
481-487: Panic safety doc aligns with implementation.The callback panic-safety notes reflect the session layer’s defer/recover behavior previously discussed. Looks good.
If desired, we can add a brief note that logs include stack traces to make expectations explicit.
README.md (2)
481-487: Panic safety description looks correct.Matches the known defer/recover behavior in session close path. Good.
251-254: Clarify partial write semantics of WritePkg.You document returning totalBytesLength and sendBytesLength. Add one sentence in this section clarifying behavior on partial writes and how callers should react (e.g., retry strategy).
I can draft a short paragraph if you confirm the current WritePkg behavior on partial OS-buffer writes (queueing vs synchronous send).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (16)
README.md (8)
35-53: Add fenced code language for ASCII diagram.Specify a language to satisfy MD040. Use text for diagrams.
-``` +```text ┌─────────────────────────────────────────────────────────────┐ │ Application Layer │ ├─────────────────────────────────────────────────────────────┤ ... -``` +```
65-66: Fix heading level increment (H2 → H4).Change to H3 to satisfy MD001.
-#### Complete Data Flow Diagram +### Complete Data Flow Diagram
67-78: Add fenced code language for second diagram.Same MD040 fix as above.
-``` +```text ┌─────────────────────────────────────────────────────────────────────────────┐ │ Incoming Data Flow │ ... -``` +```
80-89: Use headings instead of bold labels.Replace emphasis-as-heading per MD036. Apply similarly to other bold “section labels” in this doc (e.g., Key Methods, Connection Management, Configuration Settings, Handler Settings, Data Transmission, Attribute Management, Statistics, Automatic Active Time Updates, Server-Side Heartbeat Detection, Active Time Update Timeline, Key Points, Basic Configuration, WebSocket Configuration, TLS Configuration, Basic Configuration, Reconnection Configuration, Certificate Configuration, Configuration Examples, Key Features, Usage Example, Callback Management, Type Requirements).
-**Processing Order:** +### Processing Order ... -**Key Components:** +### Key Components
118-127: Length-prefixed decode: add bounds checks to avoid OOM/overflow.Guard unreasonable lengths and prevent overflow before slicing.
- // 2. Parse packet length - length := int(data[0])<<24 | int(data[1])<<16 | int(data[2])<<8 | int(data[3]) + // 2. Parse packet length (big-endian) + length := int(data[0])<<24 | int(data[1])<<16 | int(data[2])<<8 | int(data[3]) + // 2.1. Basic sanity checks (avoid OOM/overflow with malicious input) + const maxPacket = 4 * 1024 * 1024 // 4MB demo cap; tune per app + if length <= 0 || length > maxPacket { + return nil, 0, fmt.Errorf("invalid packet length: %d", length) + } // 3. Check if we have complete packet - if len(data) < 4+length { + if len(data) < 4+length { return nil, 0, nil // Incomplete packet, wait for more data }
180-183: Handle WritePkg error.Log or act on send failure for clearer examples.
- session.WritePkg(response, time.Second*5) + if _, _, err := session.WritePkg(response, 5*time.Second); err != nil { + log.Printf("send failed: %v", err) + }
366-372: Unify package qualifier (transport vs getty) across examples.Quick Start uses transport.NewTCPServer; this block uses getty.NewTCPServer. Pick one for consistency (suggest transport to match earlier).
-// Create TCP server -server := getty.NewTCPServer( - getty.WithLocalAddress(":8080"), // Listen address - getty.WithServerTaskPool(taskPool), // Task pool -) +// Create TCP server +server := transport.NewTCPServer( + transport.WithLocalAddress(":8080"), // Listen address + transport.WithServerTaskPool(taskPool), // Task pool +)
452-455: Use time.Duration for reconnect interval (current API uses int as nanoseconds)
- Verified: WithReconnectInterval(reconnectInterval int) stores an int; defaultReconnectInterval = 3e8 // 300ms and reConnect casts to time.Duration, so the unit is nanoseconds (README is correct).
- Change: make ClientOptions.reconnectInterval time.Duration and WithReconnectInterval(interval time.Duration); set defaultReconnectInterval = 300*time.Millisecond; update call sites/tests that pass numeric literals (e.g. transport/server_test.go, transport/options_test.go, transport/client_test.go, examples) and fix the incorrect comment in transport/options.go.
README_CN.md (8)
35-53: 为 ASCII 图添加代码块语言。满足 MD040,建议使用 text。
-``` +```text ┌─────────────────────────────────────────────────────────────┐ │ 应用层 (Application Layer) │ ... -``` +```
65-66: 修正标题层级(H2 → H4 跳级)。改为 H3 以满足 MD001。
-#### 完整数据流图 +### 完整数据流图
67-78: 为第二个图的代码块添加语言。同上,使用 text。
-``` +```text ┌─────────────────────────────────────────────────────────────────────────────┐ │ 接收数据流 │ ... -``` +```
80-89: 用标题替换加粗小节标签。按 MD036 将强调改为标题。请在全文其他类似位置同步处理(例如:主要方法说明、连接管理、配置设置、处理器设置、数据发送、属性管理、统计信息、自动活跃时间更新、服务端心跳检测、活跃时间更新时间线、关键要点、基础配置/WebSocket 配置/TLS 配置、基础配置/重连配置/证书配置、配置示例、主要特性、使用示例、回调管理、类型要求)。
-**处理顺序:** +### 处理顺序 ... -**关键组件:** +### 关键组件
118-127: 长度前缀解包:补充边界校验避免 OOM/溢出。在切片前校验 length 合理性。
- // 2. 解析数据包长度 - length := int(data[0])<<24 | int(data[1])<<16 | int(data[2])<<8 | int(data[3]) + // 2. 解析数据包长度(大端) + length := int(data[0])<<24 | int(data[1])<<16 | int(data[2])<<8 | int(data[3]) + // 2.1 合理性校验(防止恶意长度导致 OOM/溢出) + const maxPacket = 4 * 1024 * 1024 // 演示上限 4MB;按需调整 + if length <= 0 || length > maxPacket { + return nil, 0, fmt.Errorf("invalid packet length: %d", length) + } // 3. 检查是否有完整的数据包 - if len(data) < 4+length { + if len(data) < 4+length { return nil, 0, nil // 数据包不完整,等待更多数据 }
180-183: 发送返回值需处理错误。示例中处理 WritePkg 错误更清晰。
- session.WritePkg(response, time.Second*5) + if _, _, err := session.WritePkg(response, 5*time.Second); err != nil { + log.Printf("发送失败: %v", err) + }
366-372: 统一包前缀(transport vs getty)。快速开始用的是 transport.NewTCPServer;此处为 getty.NewTCPServer。建议统一(此处改为 transport)。
-// 创建 TCP 服务器 -server := getty.NewTCPServer( - getty.WithLocalAddress(":8080"), // 监听地址 - getty.WithServerTaskPool(taskPool), // 任务池 -) +// 创建 TCP 服务器 +server := transport.NewTCPServer( + transport.WithLocalAddress(":8080"), // 监听地址 + transport.WithServerTaskPool(taskPool), // 任务池 +)
452-455: 确认:WithReconnectInterval 的参数是 int,表示纳秒;文档与代码一致,但建议改用 time.Duration
- 证据:transport/options.go:127 定义为 func WithReconnectInterval(reconnectInterval int);transport/client.go:464-465 将该 int 作为纳秒转换为 time.Duration;tests 中调用为 5e8(500,000,000)。
- 建议:要么将签名改为 WithReconnectInterval(d time.Duration)(更符合 Go 习惯),要么在 README 明确写出“以纳秒为单位的整数”。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)README_CN.md(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-18T08:16:03.121Z
Learnt from: ayamzh
PR: AlexStocks/getty#91
File: README_CN.md:0-0
Timestamp: 2025-09-18T08:16:03.121Z
Learning: In Getty's callback system, while callbacks.Invoke() propagates panics, the session layer in transport/session.go wraps the callback invocation in a goroutine with defer/recover to handle panics safely and log them with stack traces, making the "Panic 安全性" documentation in README accurate.
Applied to files:
README.mdREADME_CN.md
🪛 markdownlint-cli2 (0.18.1)
README.md
35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
65-65: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
67-67: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
264-264: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
270-270: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
278-278: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
284-284: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
289-289: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
294-294: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
299-299: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
324-324: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
343-343: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
431-431: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
435-435: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
441-441: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
447-447: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
452-452: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
456-456: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
463-463: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (6)
README.md (3)
190-193: Cron period unit now consistent (ms). Good.Example uses 5000 ms for 5s and comment matches. This resolves the earlier mismatch.
246-250: Session interface now includes read/write timeouts.Docs align with example usage (SetReadTimeout/SetWriteTimeout).
320-322: Active time semantics clarified.Wording now correctly states only data reception and WS ping/pong update active time; send does not.
README_CN.md (3)
192-193: Cron 周期单位一致(毫秒)。已修复。示例使用 5000 ms 对应 5 秒并与文档一致。
248-249: Session 接口已包含读/写超时方法。与示例一致,避免读者困惑。
320-322: 活跃时间更新语义已统一。明确仅“接收数据”和 WS ping/pong 更新,发送不更新。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (11)
README.md (5)
100-105: Make import alias explicit for clarity.The package at github.com/AlexStocks/getty/transport declares package name getty. Being explicit helps readers.
Apply:
-import ( +import ( "fmt" "log" "time" - "github.com/AlexStocks/getty/transport" + getty "github.com/AlexStocks/getty/transport" gxsync "github.com/dubbogo/gost/sync" )
184-187: Handle WritePkg error in example.Show best practice by logging send errors.
Apply:
- response := fmt.Sprintf("Echo: %s", string(messageData)) - session.WritePkg(response, time.Second*5) + response := fmt.Sprintf("Echo: %s", string(messageData)) + if _, _, err := session.WritePkg(response, 5*time.Second); err != nil { + log.Printf("send failed: %v", err) + }
268-298: Fix markdownlint MD036: use headings instead of bold emphasis.Convert emphasis lines to proper subheadings.
Apply:
-**Connection Management** +#### Connection Management @@ -**Configuration Settings** +#### Configuration Settings @@ -**Handler Settings** +#### Handler Settings @@ -**Data Transmission** +#### Data Transmission @@ -**Attribute Management** +#### Attribute Management @@ -**Statistics** +#### Statistics
435-448: Fix markdownlint MD036 in options sections.Use headings for sub-sections.
Apply:
-**Basic Configuration** +#### Basic Configuration @@ -**WebSocket Configuration** +#### WebSocket Configuration @@ -**TLS Configuration** +#### TLS Configuration
451-464: Fix markdownlint MD036 for client options.Same conversion as server options.
Apply:
-**Basic Configuration** +#### Basic Configuration @@ -**Reconnection Configuration** +#### Reconnection Configuration @@ -**Certificate Configuration** +#### Certificate ConfigurationREADME_CN.md (6)
100-105: 建议显式设置 import 别名。路径为 transport,但包名为 getty;显式别名更直观。
Apply:
-import ( +import ( "fmt" "log" "time" - "github.com/AlexStocks/getty/transport" + getty "github.com/AlexStocks/getty/transport" gxsync "github.com/dubbogo/gost/sync" )
184-187: 在示例中处理 WritePkg 错误。体现最佳实践。
Apply:
- response := fmt.Sprintf("Echo: %s", string(messageData)) - session.WritePkg(response, time.Second*5) + response := fmt.Sprintf("Echo: %s", string(messageData)) + if _, _, err := session.WritePkg(response, 5*time.Second); err != nil { + log.Printf("发送失败: %v", err) + }
268-300: 修复 markdownlint MD036:用标题替代加粗文本。将强调行改为子标题。
Apply:
-**连接管理** +#### 连接管理 @@ -**配置设置** +#### 配置设置 @@ -**处理器设置** +#### 处理器设置 @@ -**数据发送** +#### 数据发送 @@ -**属性管理** +#### 属性管理 @@ -**统计信息** +#### 统计信息
435-448: 修复 Options 小节的 MD036。同样改为子标题。
Apply:
-**基础配置** +#### 基础配置 @@ -**WebSocket 配置** +#### WebSocket 配置 @@ -**TLS 配置** +#### TLS 配置
451-464: 修复客户端选项的 MD036。统一为标题。
Apply:
-**基础配置** +#### 基础配置 @@ -**重连配置** +#### 重连配置 @@ -**证书配置** +#### 证书配置
553-553: 末尾空行/标点一致性。可考虑与英文版一致保留“Apache License 2.0”英文本或在两端采用同一格式。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)README_CN.md(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-18T08:16:03.121Z
Learnt from: ayamzh
PR: AlexStocks/getty#91
File: README_CN.md:0-0
Timestamp: 2025-09-18T08:16:03.121Z
Learning: In Getty's callback system, while callbacks.Invoke() propagates panics, the session layer in transport/session.go wraps the callback invocation in a goroutine with defer/recover to handle panics safely and log them with stack traces, making the "Panic 安全性" documentation in README accurate.
Applied to files:
README_CN.mdREADME.md
🪛 markdownlint-cli2 (0.18.1)
README_CN.md
268-268: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
274-274: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
282-282: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
288-288: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
293-293: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
298-298: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
303-303: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
328-328: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
347-347: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
435-435: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
439-439: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
445-445: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
451-451: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
456-456: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
460-460: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
467-467: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
README.md
268-268: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
274-274: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
282-282: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
288-288: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
293-293: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
298-298: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
303-303: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
328-328: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
347-347: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
435-435: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
439-439: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
445-445: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
451-451: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
456-456: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
460-460: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
467-467: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (10)
README.md (5)
196-196: Good fix: SetCronPeriod unit now consistent (ms).Using 5000 with "5 second" comment aligns example with documented ms unit.
248-254: Session interface now includes read/write timeouts.SetReadTimeout/SetWriteTimeout are documented and match usage in examples.
324-326: Active-time wording consistent with behavior.Explicitly states TCP/UDP send does not update active time; only data reception and WS ping/pong do.
332-334: Cron comment aligned with active-time semantics.“data reception or WS ping/pong” phrasing removes the previous ambiguity.
456-459: Confirmed: WithReconnectInterval uses nanoseconds. DefaultReconnectInterval = 3e8 (300ms) and the configured int is ultimately converted to time.Duration (used with gxtime.After), so the README's "nanoseconds" unit is correct.README_CN.md (5)
196-196: 已修正:SetCronPeriod 单位与文档一致(毫秒)。示例使用 5000 并注明 5 秒,和“毫秒”说明一致。
252-253: 接口已补全读取/写入超时方法。与示例一致,避免读者困惑。
324-326: 活跃时间语义已统一。明确 TCP/UDP 发送不更新活跃时间,仅接收数据与 WS ping/pong 更新。
332-334: OnCron 注释与语义一致。“接收数据或 WS ping/pong”去除了“发送”歧义。
456-459: 确认:WithReconnectInterval 单位为“纳秒”,文档描述正确。 实现中 reconnectInterval 以 int 表示并在 reConnect 中转换为 time.Duration(默认 defaultReconnectInterval = 3e8 // 300ms),README 中“纳秒”与实现一致。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
README.md (2)
239-266: Session interface: add GetActive()/UpdateActive() to match examples and narrative.Examples use session.GetActive(), and intro text says Session.UpdateActive exists. Add both to the interface listing to avoid confusion.
Apply:
type Session interface { Connection Reset() Conn() net.Conn Stat() string IsClosed() bool EndPoint() EndPoint SetMaxMsgLen(int) SetName(string) SetEventListener(EventListener) SetPkgHandler(ReadWriter) SetReader(Reader) SetWriter(Writer) + GetActive() time.Time + UpdateActive() SetCronPeriod(int) SetWaitTime(time.Duration) SetReadTimeout(time.Duration) SetWriteTimeout(time.Duration) GetAttribute(any) any SetAttribute(any, any) RemoveAttribute(any) WritePkg(pkg any, timeout time.Duration) (totalBytesLength int, sendBytesLength int, err error) WriteBytes([]byte) (int, error) WriteBytesArray(...[]byte) (int, error) Close() AddCloseCallback(handler, key any, callback CallBackFunc) RemoveCloseCallback(handler, key any) }Also consider noting that RemoteAddr() comes via the embedded Connection (used in examples).
269-305: markdownlint MD036: replace bold “pseudo-headings” with real headings.Convert emphasized labels to proper headings to satisfy MD036 and improve navigation.
Apply:
-**Connection Management** +##### Connection Management @@ -**Configuration Settings** +##### Configuration Settings @@ -**Handler Settings** +##### Handler Settings @@ -**Data Transmission** +##### Data Transmission @@ -**Attribute Management** +##### Attribute Management @@ -**Statistics** +##### StatisticsAlso applies to: 330-330, 349-349
README_CN.md (2)
239-266: Session 接口需补充 GetActive()/UpdateActive()。示例使用 session.GetActive(),开头叙述也称 Session 提供 UpdateActive。为避免读者困惑,请在接口列表中加入这两个方法。
type Session interface { Connection Reset() Conn() net.Conn Stat() string IsClosed() bool EndPoint() EndPoint SetMaxMsgLen(int) SetName(string) SetEventListener(EventListener) SetPkgHandler(ReadWriter) SetReader(Reader) SetWriter(Writer) + GetActive() time.Time + UpdateActive() SetCronPeriod(int) SetWaitTime(time.Duration) SetReadTimeout(time.Duration) SetWriteTimeout(time.Duration) GetAttribute(any) any SetAttribute(any, any) RemoveAttribute(any) WritePkg(pkg any, timeout time.Duration) (totalBytesLength int, sendBytesLength int, err error) WriteBytes([]byte) (int, error) WriteBytesArray(...[]byte) (int, error) Close() AddCloseCallback(handler, key any, callback CallBackFunc) RemoveCloseCallback(handler, key any) }另外可注明 RemoteAddr() 来自嵌入的 Connection(示例中有使用)。
270-283: markdownlint MD036:将加粗“小标题”改为真实标题。把强调文本的小节标题改为 Markdown 标题,便于导航并满足 MD036。
-#### 连接管理 +##### 连接管理 @@ -#### 配置设置 +##### 配置设置 @@ -#### 处理器设置 +##### 处理器设置 @@ -#### 数据发送 +##### 数据发送 @@ -#### 属性管理 +##### 属性管理 @@ -#### 统计信息 +##### 统计信息Also applies to: 284-289, 290-302
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)README_CN.md(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-18T08:16:03.121Z
Learnt from: ayamzh
PR: AlexStocks/getty#91
File: README_CN.md:0-0
Timestamp: 2025-09-18T08:16:03.121Z
Learning: In Getty's callback system, while callbacks.Invoke() propagates panics, the session layer in transport/session.go wraps the callback invocation in a goroutine with defer/recover to handle panics safely and log them with stack traces, making the "Panic 安全性" documentation in README accurate.
Applied to files:
README.mdREADME_CN.md
🪛 markdownlint-cli2 (0.18.1)
README.md
270-270: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
276-276: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
284-284: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
290-290: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
295-295: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
300-300: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
305-305: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
330-330: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
349-349: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
437-437: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
441-441: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
447-447: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
453-453: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
458-458: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
462-462: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
469-469: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
README_CN.md
437-437: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
441-441: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
447-447: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
453-453: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
458-458: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
462-462: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
469-469: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (8)
README.md (4)
196-200: Good fix: heartbeat period units aligned (ms).Using SetCronPeriod(5000) with “5 second heartbeat detection” resolves the prior unit mismatch.
326-362: Active-time semantics now consistent.Docs clearly state only data reception and WS ping/pong update active time; TCP/UDP send does not. This resolves earlier contradiction.
487-497: Callback section content looks correct (panic safety).Matches transport/session.go behavior (callbacks run in a goroutine with defer/recover).
Also applies to: 516-521
458-461: Confirmed: WithReconnectInterval takes int interpreted as nanosecondsDefinition is func WithReconnectInterval(reconnectInterval int) (transport/options.go); defaultReconnectInterval = 3e8 (300ms) and tests use values like 5e8 — README's "nanoseconds" and the int signature are correct.
README_CN.md (4)
196-200: 心跳周期单位(毫秒)已对齐。使用 SetCronPeriod(5000) 与“5秒心跳检测”注释一致,修复了之前的单位不一致问题。
326-365: 活跃时间更新语义已统一。明确仅“接收数据”和 WebSocket ping/pong 更新活跃时间;发送不会更新。与示例一致。
491-497: 回调小节关于 Panic 安全性的描述正确。符合 session 层 defer/recover 的实现。
Also applies to: 516-521
458-461: 确认:WithReconnectInterval 为 int,文档“纳秒”描述正确。源码 transport/options.go 中为 int(ClientOptions.reconnectInterval);transport/client.go 中 reConnect 将其按纳秒参与计算并转换为 time.Duration(tests 使用 5e8),README/README_CN 的“纳秒”与实现一致。
What this PR does:
This PR comprehensively restructures and enhances the Getty framework documentation to improve readability and user experience. The changes include:
Which issue(s) this PR fixes:
Summary by CodeRabbit