Skip to content

style: 统一注释术语,去掉历史叙述与装饰标记 - #259

Merged
NeverENG merged 2 commits into
mainfrom
refactor/comment-style
Aug 12, 2026
Merged

style: 统一注释术语,去掉历史叙述与装饰标记#259
NeverENG merged 2 commits into
mainfrom
refactor/comment-style

Conversation

@NeverENG

@NeverENG NeverENG commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

承接 #250(目录布局)、#253(标识符命名)。本批只改注释——diff 已逐行核对,每条改动行都落在注释内,未动一行代码。

一、术语统一:同一概念只有一个名字

此前同一概念有两种叫法,甚至同一行内并存(// 写放大 = (flush + compaction) / 刷盘「flush(flush → L0)」)。

概念 改前(中/英散文) 改后
flush 37 / 16 全用 flush(与 Flush 方法同名)
memtable 2 / 13 全用 memtable
compaction 13 / 21 全用 compaction
墓碑 34 / 0 全用中文(本无混用)
分片 96 / 2 全用中文(本无混用),2 处英文改回中文

一处需要说明的偏离

这与我先前提出的「凡有代码实体就用英文」的字面规则有两处不同:墓碑与分片在注释里本就一致(34:0、96:2),按字面执行等于为 2 处英文去改 96 处中文,把本来一致的地方改成不一致。故按该规则的目标(一致且好读)而非字面执行。

而且先前那组「五五混用」的数字本身是错的:我用了 grep -i,把注释里的标识符引用ShardCountMemTable)也计入了英文散文。案例:shard 在注释散文里实际只出现 2 次,不是 32 次。真实混用只在 flush / compaction / memtable 三处。

「合并」刻意保留 3 处:ScanRangeSnapshotLive 的 active+dirty 归并、TestMergeBasic 的多路归并——那是 k 路 merge,与 compaction 是不同概念,不该合并成一个词。

二、去掉 24 处叙述历史的注释

「此前……」「原先 Type 为裸 string……」这类内容属于 git 历史,不属于代码:注释应陈述当前的不变量与理由。这同时修正我自己的前后不一致——先前提出该原则后又大量写入。

示例:

改前  // 【并发正确性】active 的查找必须在锁内完成。此前的实现只在锁内拷贝表指针便释放
      // 锁,随后无锁遍历跳表——而 Put/Delete 正持写锁改写同一张 active 的节点指针…
      // 只圈住内存查找、不圈住 SSTable 查找:后者要读磁盘,若也持锁会让写入停等 I/O。

改后  // active 的查找必须持锁:Put/Delete 会并发改写其节点指针,无锁遍历可能跟到只连了
      // 一半的新节点,读出错值甚至解引用空指针。SSTable 查找不持锁——它要读磁盘,持锁会
      // 让写入停等 I/O;dirty 一经交换便不再被写入,故也无需持锁。

三、去掉 7 处 【】 装饰标记

Go 生态无此写法,改为普通行文。

四、修 3 处文档注释首行不以标识符开头

Go 强约定。其中 NewKVServer 的注释首行仍写着重命名前的旧名 NewFSM —— 实质的文档缺陷。

过程记录

首次尝试时加了「压缩连续空格」规则,破坏了 7 个文件中 28 行 Go 文档列表的缩进(// - 被压成 // - ,续行也失去对齐)。已回退重做,并加了校验:改动前后带缩进的注释行数必须相等。另在汉字与拉丁字母/数字之间统一补空格(32 行),行内注释(code // 注释)用引号感知的扫描单独处理,避免把 "http://" 误判为注释。

验证

go build ./...go vet ./...go test -race ./...gofmt 全绿。

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation

    • Clarified that the public interface is provided through the client package and BanNet protocol.
    • Documented that gRPC transport is internal and external integrations should use supported SDKs or gateways.
    • Improved guidance for bounded retries, status codes, concurrency safety, persistence, flushing, snapshots, and recovery.
    • Standardized terminology across storage, delivery, Raft, and service documentation.
  • Tests

    • Added regression coverage for standalone gRPC put and get operations.

NeverENG and others added 2 commits August 13, 2026 02:18
把 .proto 交给使用方自行生成客户端,等于把内部传输实现当成公开 API:使用方要装 protoc
工具链、要理解 protobuf,还得跟着我们的 proto 变更走。对外契约应当是我们自己的接口,
gRPC 顶多是它背后的一种传输。

kvgrpc 移入 internal/kvgrpc。Go 的 internal/ 由编译器强制——模块外无法导入,故该边界不
依赖口头约定。补 package 文档说明其定位,并在 README 明确「对外契约只有 client 包与
BanNet 协议规范,其余包均为内部实现」。多语言接入的正确做法是在 BanNet 协议之上提供各
语言 SDK 或网关,而非暴露 protobuf。

顺带说明:Go SDK 本身与 gRPC 无关,一行都不碰——BanNet 协议是自有的,帧编解码手写,
零代码生成。protoc-gen-go 仅在改动 .proto 后重新生成 .pb.go 时才需要,属这条 gRPC
路径自带的维护负担,不是能力缺口。go_package 选项已同步为新路径。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
四项,全部只改注释,不动一行代码(diff 已逐行核对:每条改动行都落在注释内)。

一、术语统一。同一概念此前有两种叫法,甚至同一行并存(如「flush(flush → L0)」):

              改前(中/英)   改后
  flush        37 / 16      全用 flush(与 Flush 方法同名)
  memtable      2 / 13      全用 memtable
  compaction   13 / 21      全用 compaction
  墓碑         34 / 0       全用中文(本无混用)
  分片         96 / 2       全用中文(本无混用),2 处英文改回中文

需要说明的是,这与我先前给出的「有代码实体就用英文」的字面规则有两处不同:墓碑与分片
在注释里本就一致(34:0、96:2),按字面执行等于为 2 处英文去改 96 处中文,把本来一致的
地方改成不一致。故按该规则的目标(一致且好读)而非字面执行。先前所谓「五五混用」的数字
是 grep -i 把注释里的标识符引用(ShardCount、MemTable)也计入所致,实际混用只在
flush / compaction / memtable 三处。

「合并」保留 3 处:ScanRange 与 SnapshotLive 的 active+dirty 归并、TestMergeBasic 的
多路归并——那是 k 路 merge,与 compaction 是不同概念。

二、去掉 24 处叙述历史的注释(「此前……」「原先……」)。代码注释应陈述当前的不变量与
理由,曾经是什么样属于 git 历史。这也修正我自己的前后不一致:先前提出该原则后又大量写入。

三、去掉 7 处【】装饰标记,改为普通行文(Go 生态无此写法)。

四、修 3 处文档注释首行不以标识符开头(Go 强约定),其中 NewKVServer 的注释首行仍写着
重命名前的旧名 NewFSM。

另补:在汉字与拉丁字母/数字之间统一补空格(32 行)。首次尝试时用了「压缩连续空格」的
规则,破坏了 7 个文件里 28 行 Go 文档列表的缩进(「//   - 」被压成「// - 」),已回退
重做并加校验:改动前后带缩进的注释行数须相等。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@NeverENG
NeverENG merged commit 8237d00 into main Aug 12, 2026
3 checks passed
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e60d9121-22ba-4117-8614-37019ada767e

📥 Commits

Reviewing files that changed from the base of the PR and between 5336b76 and 09da360.

⛔ Files ignored due to path filters (2)
  • internal/kvgrpc/kv.pb.go is excluded by !**/*.pb.go
  • internal/kvgrpc/kv_grpc.pb.go is excluded by !**/*.pb.go
📒 Files selected for processing (38)
  • README.md
  • bannet/conn_manager.go
  • client/client.go
  • cmd/ban-bench-grpc/runner.go
  • cmd/ban-grpc-server/main.go
  • cmd/ban-ingest/main.go
  • config/global.go
  • internal/kvgrpc/grpc_client.go
  • internal/kvgrpc/grpc_server.go
  • internal/kvgrpc/grpc_server_test.go
  • internal/kvgrpc/kv.proto
  • pkg/admission/limiter.go
  • pkg/metrics/metrics.go
  • pkg/proto/codes.go
  • raft/raft.go
  • raft/raft_wal.go
  • raft/rpc.go
  • service/checkpoint_test.go
  • service/cluster/routing.go
  • service/delivery/file_sink.go
  • service/delivery/idempotent_sink.go
  • service/fsm.go
  • service/fsm_test.go
  • service/shardkv/shardkv.go
  • service/shardkv/shardkv_test.go
  • storage/compaction_bench_test.go
  • storage/compaction_stats.go
  • storage/engine.go
  • storage/engine_test.go
  • storage/errors.go
  • storage/memtable.go
  • storage/memtable_test.go
  • storage/merge_test.go
  • storage/recency_test.go
  • storage/reload_recover_test.go
  • storage/sstable.go
  • storage/wal.go
  • storage/wal_bench_test.go

📝 Walkthrough

Walkthrough

The change adds an internal gRPC client and updates its package wiring and standalone test. It revises the public API documentation and clarifies existing storage, persistence, protocol, runtime, and test comments without changing their behavior.

Changes

API boundary and documentation

Layer / File(s) Summary
Internal gRPC transport
internal/kvgrpc/*, cmd/ban-*-grpc/*, README.md
Adds gRPC Put, Get, and Delete client methods. Updates protobuf and command package paths. Documents gRPC as an internal transport.
Storage and flush contracts
storage/*, service/checkpoint_test.go, service/fsm.go, config/global.go, pkg/admission/limiter.go, pkg/metrics/metrics.go
Clarifies flush, WAL, checkpoint, compaction, locking, recovery, and ordering terminology.
Runtime and protocol documentation
bannet/conn_manager.go, client/client.go, raft/*, service/cluster/routing.go, service/delivery/*, service/shardkv/*, cmd/ban-ingest/main.go, pkg/proto/codes.go
Clarifies existing retry, concurrency, Raft, routing, delivery, shard lifecycle, metric, and status semantics.

Estimated code review effort: 3 (Moderate) | ~20 minutes

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/comment-style

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

🐯 BanGD 数据库内核评审

整体风险:🟢 低

变更总结:纯注释与文档术语统一 PR:把 flush/memtable/compaction 的中英文混用统一为单一命名,去掉叙述历史的注释(改为陈述当前不变量与理由),移除 【】 装饰标记,修正文档注释首行不以标识符开头的问题,并重命名 kvgrpc 包路径为 internal/kvgrpc(含 README、package 注释、proto go_package 的配套更新),使内部传输在编译期被隔离而非依赖口头约定。全程未动任何可执行代码行。

需要特别指出:虽然标题自称「只改注释」,但 kvgrpc → internal/kvgrpc 的目录迁移涉及 import 路径变更与 proto 文件 go_package 的修改,会触发 kv.pb.go/kv_grpc.pb.go 的重新生成路径。这属于接口边界变更,虽然不改变运行时行为,但影响外部使用者(此前有引用 kvgrpc 的用户会因 internal 隔离而无法编译),且新 package 文档注释新增了大量关于「对外契约」的论断(README 也新增了「对外契约只有两样」整节)——这已经超出了「纯注释术语统一」的宣称范围,属于一次公开 API 边界收缩决策被顺带塞进了注释 PR。

本评审不阻塞合入;架构级建议以 Issue 形式跟踪,普通问题在下方内联列出。

架构问题(共 2 项)

普通问题(共 1 项)

💡 [建议 · 注释精度] service/shardkv/shardkv.go:161 「分片 组」中英混杂失当

  • // ShardLeader 返回 分片 组在本节点视角的 leader 地址——把「分片」夹入中文句子中,与前文统一使用英文代码实体名的原则不符:ShardLeader 是代码实体,但「分片」在此是中文名词而非代码实体引用。读取体验上「分片 组」在中文里既不自然也不正确。
  • 建议:改为「返回该分片组在本节点视角的 leader 地址」或「返回 shard 组(第 shardID 号分片组)在本节点视角的 leader 地址」。

本次评审消耗 token:共 232637 tokens(输入 217983,输出 3774,缓存命中 10880,缓存写入 0)|维度 [concurrency, memory, lock, storage, schema]|补充阅读周边文件 [internal/kvgrpc/grpc_client.go, bannet/server.go, client/conn.go, client/errors.go]|对抗式复核 3 票/条,过滤疑似误报 1 条

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