Conversation
Kripu77
left a comment
Kripu77
left a comment
There was a problem hiding this comment.
Review of PR 56 for issue 49.
Behavior is mostly there: detect/keep, github or linear with a team key, write through config_tracker and config_write_conventions, review then confirm, no-TTY does not write, --yes is refused. Tests cover those paths. I am not approving the implementation.
factory.sh went 1138 to 1429. The config writers belong here (PR 46). The TTY wizard does not. PR 27 already refused this file crossing 1k. Source the installer.
Detect finishes before the spinner. Confirm then clears the review for a 5/5 Write screen. Typing any skill line replaces the whole conventions file, and the Skills stage never shows what was already there.
Inline comments have the rewrites. Do not merge until those land.
GitHub will not let this account request changes on its own PR, so this is a comment review. Verdict is still request changes.
Pull the TTY installer out of factory.sh, detect config under the spinner, show current skill lines, and write from the review screen.
Kripu77
left a comment
Kripu77
left a comment
There was a problem hiding this comment.
Review of PR 56 for issue 49, second pass on b168773.
Previous request-changes items landed. The TTY wizard lives in lib/setup.sh. factory.sh is 1138 to 1192: dispatcher, config_read, and the symlink resolve so factory setup can source the lib. Detect I/O sits under the spinner. Review is the last numbered stage; confirm writes in place. Skills shows current lines; empty keeps them. install.sh still names the runner.
Tests cover detect, keep, skip, cancel, write-through-config, no-TTY, and --yes. I ran tests/setup.sh, tests/config.sh, tests/conventions.sh, and tests/install.sh; they passed.
GitHub will not let this account approve its own PR, so this is a comment review. Verdict is still approve. CI next. Do not merge until checks are green. A person merges.
There was a problem hiding this comment.
I would much prefer to write the CLI in go like here
https://github.com/urfave/cli
2 participants
factory setupis the human path over the existingfactory.sh configwriters: detect, tracker, skills, review, then write. No TTY prints current config and exits; cancel and keep leave files unchanged. Pattern matches PR 46 (factory.sh config).Implements #49.