Repository navigation
refactor: code consistency pass against cella house style - #41
Merged
Merged
Conversation
- drop getRemoteUrl, FileChange.newPath, AnalyzedFile.hasConflict, AnalysisSummary.total and the write-only audit fields - remove the unreachable identical fallback in analyze-core and the dead pinned-rename fallback in merge-engine - delete runStats' unused json option, tests/integration/FIXTURE-REPO.md, the unused vitest globals flag and the src test include glob - unexport 20 file-local declarations; none had outside consumers - typecheck the published config.ts entry via tsconfig include - reword the syncWithPackages JSDoc and runSync's caller note to match reality Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Duplication consolidated: - one private pnpm-JSON runner behind getOutdatedPackages/runPnpmAudit - SYNC_APPLIED_STATUSES shared by merge-engine totalResolved and sync's hasStagedSyncChanges; PACKAGE_JSON_SYNC_KEYS spells the 10 keys once (type and zod schema derive from it) - parseSyncManifest shared by readSyncManifest and readManifestAtRef - runEngineWithSpinner and printEngineReports/printLogFileReport replace the engine-call and report-tail blocks duplicated by analyze and sync - audit: one severity-count formatter, vuln icons as a record lookup, DIVIDER instead of inline rules - isUpstreamRepo shared by the menu context and contributions - COMMIT_LIST_MAX (50-commit display cap), MENU_DIVIDER and MAX_BUFFER (50MB subprocess buffer) each defined once - errorMessage(error) replaces seven inline instanceof-Error ternaries (non-Error throws now stringify instead of 'unknown error') - display's checkMark replaces inline pc.green checkmarks - CommitRangeEntry in config/types used by git, display, merge-engine, sync and MergeResult - forks reuses getWorkingTreeChangeCount (wrapped in catch) - cli readOptions collapses its ladders onto str()/flag(); option literals shared verbatim across services are hoisted Structure unified: - runAudit and runStats take the RuntimeConfig like every service; cella-cli switch cases consistently braced - preflight and getMenuContext are synchronous - git: fetch renamed fetchRemote (shadowed global fetch), merge hardcodes --no-commit/--no-edit (every caller passed both), listCommitsBetween drops oldestFirst and paginates with slice, getFileChanges dedupes the zero-hash ternary - merge-engine: isProtectedFile predicate ties the batch plan to the phase-5 skip; protected-conflict membership test via a Set - contributions: toItem closure, selectFork extraction, constant bounds memo inlined - audit: promptForUpdates split into buildUpdateChoices and executeUpdates, workspace map flattened to Map<string, string[]>, dependent package.json reader hoisted to module level - stats: PackageStats type alias; sync: printFinishSteps inlined - display: printSummary maxCount derived from statusOrder, stranded flag-warnings JSDoc moved onto printFlagWarnings Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- contributions: unknown --fork, no valid forks, and multiple forks without --fork in non-interactive mode now throw instead of console.error + return (exit 0) - forks: unknown --fork throws too - contributions --diff with an unknown path throws like analyze --diff, so one mechanism reports all invalid targets via main's handler Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- --list now routes header/steps/warnings to stderr exactly as --json and --diff do - the machine lines themselves bypass the redirect: analyze's path-per-line loop and contributions' tab-separated rows write via writeStdout - contributions keeps its comparison banner suppressed under --list/--diff to hold the stderr noise budget; comment updated Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- one canonical phrasing per shared flag: --fork hoisted as forkOption, contributions reuses jsonOption, --diff gets the same core wording in analyze and contributions - contributions --list description now says tab-separated rows, not one file per line; the readme contributions section names the columns - sync one-liner unified on the accurate form (fresh branch, package.json sync, squash-merge PR); the menuDescription override folded in and dropped - forks, contributions, audit and -h descriptions aligned between cli.ts and readme; audit spells out 'and' - types.ts: the diff field doc covers analyze too, not contributions only - readme contributions section: a single fork is selected, not one or more Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Audit's --list skips interactive prompts; it has no machine stdout payload, so routing its report to stderr left stdout empty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- tests/helpers/test-env.ts is the one home for exec, write, commitAll, createRepo, GIT_USER and UPSTREAM_REMOTE; the ~14 local exec copies and 7 createRepo variants are deleted - packages.test.ts builds its upstream+fork pair with the shared createTestEnv, whose initial upstream files are now parameterizable - the three ensure-sync-base/graft e2e files import the shared scaffolding instead of carrying verbatim copies - the duplicated pnpm/gh spawnSync stub lives in tests/helpers/mock-pnpm-gh.ts; it stays a standalone module because a vi.mock factory importing anything that pulls node:child_process deadlocks the mock resolution - sync-gates and sync-worktree run the full sync against real repo pairs, so they move to tests/e2e/ next to the suites they belong with Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- the 88 "should <verb> X" it-titles become the suite's dominant verb-first "<verb>s X" form (negations as "does not" / "never"), across audit, git-parsing, overrides, packages and sync-e2e Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- narration: deleted ~115 comments that restated the adjacent code, trimming the
ones that held a constraint down to that constraint
- history: rewrote the 12 'sync CLI v2' file headers as noun-phrase module
descriptions; purged body vestiges (preflight's 'no longer cares', the old
VS Code diff reference, the upstream-view-worktree line, audit's catalogMode
story, the 'previously' comments in git/sync/merge-engine); the documented
compat contracts (LEGACY_CONFIG_FILE, legacy worktree sweep, fileLinkMode
'local', migrate's 'no longer upstream' label) stay
- punctuation: removed all 108 em dashes in src/ and 10 in README (punctuation
only), plus the hyphen-dashes used the same way; reworded ~20 'instead of /
rather than / as opposed to' comments to state current behavior
- jsdoc: deleted empty-calorie docs (getCurrentBranch, main, spinner one-liners,
stats format helpers, runAudit/runForks/runAnalyze duplicates, ...), reworded
'Gets X' / imperative one-liners to noun phrases, fixed runContributions'
stale plural, and documented compareVersions, hasEnv and getEnvSnapshot
- naming: sanitizeCredentials -> redactUrlSecrets, comments now name the proof
(tokens/passwords embedded in remote URLs)
- mechanical: section banners dropped or replaced with plain one-liners,
'utf-8' -> 'utf8' (17 sites), import * as fs/path -> named imports (audit,
audit-utils, coverage-utils), packages' console.warn x4 -> console.info with
pc.yellow, ~10 user messages lowercased ('sync merge staged', 'counted raw
lines of code', 'interrupted (SIGINT)', ...), cherrypick -> cherry-pick,
'&' -> 'and', the sync/forks 'command' prose now says 'service'
- tests: one assertion updated to the new 'nothing to sync:' punctuation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…review Brings in stats --since (#42). Conflicts resolved by keeping this branch's structure and wording and adding #42's feature on top: - src/cli.ts: since/md read through the str()/flag() helpers; --md joins the machine-output modes next to the audit --list exception - src/cella-cli.ts: braced stats case, runStats(config) for the snapshot mode, runBranchStats for --since - src/services/stats.ts: this branch's comments, plus the exports stats-since.ts imports - README.md: this branch's wording, plus the --since row and section Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A consistency pass over the whole repo against cella's house style, from a 7-agent review (5 analysis + 2 adversarial verification passes). Net −828 lines with core functionality and concepts unchanged. One commit per theme:
Dead code (
cbb10b1)getRemoteUrl, write-only fields (AnalysisSummary.total,AnalyzedFile.hasConflict,FileChange.newPath, unread vulnerability/registry fields), stats' unreachablejsonoption, an unreachableelsein analyze-core, and the unreachable (and incorrect) pinned-rename fallback in merge-engine.tests/integration/FIXTURE-REPO.md.config.tsto tsconfig include so the published./configentry is typechecked.Consolidation and structure (
a06def7)getOutdatedPackages/runPnpmAuditrescue blocks; oneparseSyncManifest; sharedSYNC_APPLIED_STATUSES,PACKAGE_JSON_SYNC_KEYS,CommitRangeEntry, commit-list cap, dividers, maxBuffer anderrorMessagehelpers.run<Service>(config)(audit and stats no longer take option bags that echoed config fields).fetchrenamedfetchRemote(stops shadowing the global),merge()flags hardcoded (every caller passed the same two),listCommitsBetweenpagination simplified and its always-trueoldestFirstoption removed.Surface polish (
bdd357c,5a236db,3e041eb,b1fe49c)--fork(and the other soft-fail paths in contributions/forks) now throw and exit 1; previously they printed red and exited 0, which silently fooled tooling callers.--listis now a clean machine mode like--json: human output goes to stderr, machine rows to stdout (audit's--listkeeps its report on stdout, it only skips prompts).--listis now correctly described as tab-separated rows.Tests (
509d881,940e095)tests/helpers/test-env.tsreplaces ~15 copy-pastedexec/createRepo/write/commitAllscaffolds (including a drifted git identity in packages.test.ts); shared pnpm/gh child-process stub.sync-gatesandsync-worktreemove totests/e2e/where their full-service runs belong; 88should-style titles renamed to the suite's verb-first form.Style (
bb68251)utf8/named-import/console idioms unified, user messages lowercased,sanitizeCredentialsrenamedredactUrlSecrets.Verification
pnpm check,pnpm test(271 tests / 26 files, identical to the pre-branch baseline) andpnpm test:releaseall green; commitlint clean across the branch. The only observable behavior changes are the surface-polish items above;--jsonshapes, flags and the sync/merge semantics are untouched.🤖 Generated with Claude Code