Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (23)
Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour. 📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used🪛 ast-grep (0.45.3)scripts/tests/link-skills/install-hooks-settings.sh[error] 191-191: (eval-on-variable-bash) 🔇 Additional comments (12)
📝 WalkthroughWalkthroughThe pull request renames link-skills helpers into domain-specific namespaces. It updates command dispatch, diagnostics, path calls, manifest handling, locking, linking, hooks, installation, unlinking, and test references. Reported behavior remains unchanged. ChangesLink-skills namespace migration
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to The installer’s namespace migration has no identified behavior regression and is ready to merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Claude finished @chapati23's task in 2m 23s —— View job Review in progress
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@claude review |
|
Claude finished @chapati23's task in 4m 22s —— View job Review complete
Verdict: LGTMThis is a clean, mechanical rename with no behavior changes. Method used: extracted every function definition from both the base ( Findings roll-up:
No inline comments filed — nothing rose to an actionable finding. I could not run |
AGENTS.md asks a module's public functions to carry the module name as a prefix, and its private helpers to start with an underscore. The link-skills modules kept the names the single-file script gave them, so a call site said nothing about which module owns the work. Rename every function defined under scripts/lib/link-skills/. A function that another module or the entry point calls becomes <module>_<name>: take_lock to lock_take, release_lock to lock_release, cmd_check to check_cmd, err to output_err, canonical_path to paths_canonical, remove_link to link_remove, cmd_unlink to unlink_cmd, cmd_install_hooks to install_hooks_cmd. A function only its own module calls becomes _<module>_<name>: cmd_hook to _hook_cmd, run_link to _runtime_run_link, stamp_file to _git_stamp_file, backup_path to _install_hooks_backup_path. A name that already satisfied the rule keeps it: git_root, git_branch, git_is_dirty, git_default_branch, git_pull_command, git_behind_count, names_equal, manifest_src_of, manifest_target_of, manifest_path_usable, link_candidates, runtime_link_path, sources_path_usable, sources_is_control_path and the four config_ functions. The entry point's own functions are not module functions and keep their names: main, main_*, boot_fail, script_abs_path and resolve_symlink_path. Nothing moves between files and no body changes apart from the names. Module headers, the comments that name a function, and the harness comments follow the new names. Long header lines are rewrapped where a longer name pushed them past the column the file already used. The install-hooks settings case reads the backup function out of the module by name, so its sed and its messages name _install_hooks_backup_path. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
d2e37ff to
61fcd0a
Compare
1 participant
tl;dr
Every helper inside the skill installer now carries the name of the file it
lives in, so a reader can tell at a glance where a function comes from, and a
private helper is marked as private. What the installer does is unchanged.
Nothing changes for you.
The Problem
flat names, so a call gave no hint which file defined it.
a prefix and its private helpers start with
_. The modules did not follow it.output_haslived in the manifest module andruntime_linkwas private to the runtime module.The Solution
<module>_<name>when they are called from anotherfile and
_<module>_<name>when they are called only inside their own file.keeps
main, its helpers andboot_fail.rewrap comment lines that a longer name pushed past 80 columns.
chore/split-06-install-hooks-final. Merge only the bottom layerof the stack.
Validation
Head
61fcd0a. Gates:scripts/test-link-skills.sh152 passed / 0 failed / 0skipped ✓, the same harness under
/bin/bash3.2 152 ✓,shellcheckover everytracked
.sh✓,pnpm check:shellagainst the base branch ✓,trunk fmtonthe 23 changed files ✓.
This layer was rebased onto the parent's new head
5ef74f3, with the conflictsin
scripts/lib/link-skills/unlink.shandscripts/link-skills.shresolved infavour of the parent's logic under this layer's names, and every gate above
re-run at the new head.
Review: full diff read against the base, plus a scan for every retired name in
command position across
scripts/→ 0 stale calls, 0 findings.Security: not run. The change renames functions in a local script and adds no
input, network or privilege surface.
Not run: browser verification. There is no UI.
Deferrals
None.
scripts/tests/link-skills/link-install.shline 4 names four functionsthat no longer exist anywhere. That comment was already stale before this layer
and fixing it here would widen a rename-only diff.
🤖 Generated with Claude Code