Repository navigation
Refactor: Split cmd/agentop/cmd_bob.go to move out its JSON settings editor - #1695
Conversation
cmd_bob.go had 2100 lines. About 400 of them are a byte-level JSON editor. The editor changes one top-level key in a settings file and keeps all other bytes. It has no relation to Bob itself. This change moves code only. It changes no signature and no statement. The editor moves with its doc comments to bob_settings_json.go in the same package: - bobSetKey, bobFindMember, bobSpliceOut, bobInsertMember and their helpers - errBobDuplicateKey - bobEdit, bobWriteKey and bobWriteKeys, which write the edited file cmd_bob.go has 1700 lines now. The tests stay in cmd_bob_test.go. Check: a script parsed the package before and after the move. It compared every top-level declaration (with its doc comment) and every free comment, as text. The two sets are equal. golangci-lint gives the same findings before and after the move. Part of rossoctl#1584. Signed-off-by: Hai Huang <huang195@gmail.com> Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
huang195
changed the title
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 35 minutes. View limit details
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. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Summary
This is a clean move-only refactor. I checked the claim directly: the 400 lines removed from cmd_bob.go and the 408 lines added in bob_settings_json.go are identical and in the same order. The only extra lines are the new file's package clause and import block, minus one blank separator line. Nothing was dropped. There are 40 top-level funcs before the move and 30 + 10 after it. bobDuplicateKey and bobBackupNote stay in cmd_bob.go, as stated. encoding/json and strconv correctly leave cmd_bob.go's imports. The 2100 → 1700 line counts match.
Verified locally at a20a295 in cmd/agentop: GOWORK=off go build ./..., go vet ./... and gofmt -l are clean, and the bob tests (go test -run 'Bob|bob' .) pass.
CodeQL failure (non-blocking): the 4 go/allocation-size-overflow alerts (#352–#355, lines 54 and 300 of bob_settings_json.go) are on code that this PR only moves. It is the same make([]byte, 0, len(src)+len(member)) / len(src)+len(ins) that sits at cmd_bob.go lines 591 and 837 on main. CodeQL reports them as new only because the file path is new. Two lengths of in-memory byte slices cannot add up past int, so I suggest dismissing them as false positives so the check goes green. That decision is outside the scope of this PR.
Author: huang195 (MEMBER — maintainer)
Areas reviewed: Go (cmd/agentop)
Agent/IDE config (.claude/.vscode): none
Commits: 1 commit, all signed-off: yes
CI status: all passing except CodeQL (4 alerts on relocated pre-existing code, see above)
github-project-automation
Bot
moved this from New/ToDo
to Done
in Rossoctl Issue Prioritization
Summary
Part of #1584. This PR moves code only. It changes no signature and no statement.
cmd/agentop/cmd_bob.gohad 2100 lines. About 400 of them are a byte-level JSON editor. The editor changes one top-level key in a settings file and keeps all other bytes. It has no relation to Bob itself.The editor moves, with its doc comments, to
cmd/agentop/bob_settings_json.goin the same package:bobSetKey,bobFindMember,bobSpliceOut,bobInsertMemberand their helperserrBobDuplicateKeybobEdit,bobWriteKeyandbobWriteKeys, which write the edited filecmd_bob.gohas 1700 lines now.bobDuplicateKeyandbobBackupNotestay incmd_bob.go, because they know Bob's keys and Bob's messages.The tests stay in
cmd_bob_test.go. The issue suggests a separate test surface for the editor. To move the tests is a different change, so this PR does not do it.How to review
The only lines that are not dimmed are the new file header (package clause and imports) and the import block of
cmd_bob.go.Checks
GOWORK=off go build ./...,go vet ./...andgo test ./...pass incmd/agentop.golangci-lintgives the same findings before and after the move.CodeQL
CodeQL reports
go/allocation-size-overflowatbob_settings_json.go:54and:300. The two lines move byte for byte fromcmd_bob.go. Each line ismake([]byte, 0, len(src)+len(x)), wheresrcis the contents of a Bob settings file. The sum overflows only above 2^63 bytes, so the alert is a false positive.mainhas no alert for these lines.Assisted-By: Claude (Anthropic AI) noreply@anthropic.com