chore: desloppify code health improvements (87→92.4 strict) - #811
Open
anthony-spruyt wants to merge 32 commits into
Open
chore: desloppify code health improvements (87→92.4 strict)#811anthony-spruyt wants to merge 32 commits into
anthony-spruyt wants to merge 32 commits into
Conversation
Align naming convention with codebase standard: string construction functions use `build*` prefix, not `get*`. Renames getRepoFlag → buildRepoFlag, getOrgUrl → buildOrgUrl, getMergeStrategyFlag → buildMergeStrategyFlag across GitHub, GitLab, and ADO PR strategies. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… required SecretsConfig was identically defined in both secrets/processor.ts and cli/secrets-command.ts. Export from processor.ts and import in the CLI. SettingsReport.totals.variables and RepoChanges.variables were typed as optional but always initialized as concrete values by the builder. Make them required and remove defensive ??, ?., and ! accesses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add exhaustive default cases to 5 switch statements - Extract magic numbers into named constants (MS_PER_SECOND, MAX_ERROR_MESSAGE_LENGTH) - Remove unused import (MATCH_KEY_CANDIDATES) - Fix bare catch in encryption.ts to preserve error cause - Simplify hasAnyChanges boolean return pattern - Extract validateGitHubResourceName and checkVariableSecretOverlap helpers - Add development section to README - Add desloppify scorecard Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ories Organize the flat 15-file vcs/ directory into logical subdirectories while preserving all exports through barrel index files, so existing imports via src/vcs/index.js continue to work unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The variables merge logic in mergeSettings and mergeRawSettings used repeated Record<string,unknown> casts and multi-step destructuring to separate meta-keys (deleteOrphaned, inherit) from data entries. Extract a dedicated mergeVariablesWithMeta helper following the mergeLabels pattern: typed VariableMap interface, centralized meta-key handling, single call site in each consumer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cover all 4 exported pure functions (getUniqueFileNames, generateBranchName, formatFileNames, determineMergeOutcome) with 25 tests including happy paths and edge cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Separate processOneFile into three clear phases (compute what changed, build write result, apply/report) and extract buildWriteResult helper. This removes duplicated stat tracking across the 2x2 dry-run/real x content/mode matrix and eliminates a redundant diff recomputation in dry-run mode by reusing the already-computed diffLines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Route all cross-module imports through barrel files (index.ts) instead of importing directly from internal module files. Adds missing exports to config/index.ts (validateRawConfig, validateVariableSecretOverlaps, findMatchKey) and settings/index.ts (BaseProcessorOptions). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace direct `process.env.GH_TOKEN || process.env.GITHUB_TOKEN` with the centralized `resolveGitHubToken()` utility, matching the sync command pattern. Token is now resolved per-repo inside the loop rather than once upfront, enabling future GitHub App auth support. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The Record<string, X> & { metaKey?: Y } pattern creates contradictory
types where meta-keys become `never` (intersection of incompatible types).
Introduced named interfaces (VariablesConfig, SecretsConfig, RawVariablesMap,
RawRulesetsMap, RawLabelsMap, RawGroupFileMap, RawRepoFileMap) with index
signatures that properly accommodate both data values and boolean meta-keys.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Map lookup doesn't narrow the type, so use string cast in error message instead of never-exhaustive check. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deduplicate boolean presence flag computation between validateRawConfig and validateForSync. The four flags that are identical across both functions (hasRootFiles, hasGrpFiles, hasCondGrpFiles, hasCondGrpPR) are now computed once via computeContentPresenceFlags. The two inline group-settings checks are extracted into named functions (hasGroupSettingsPresent, hasGroupSettingsActionable) that make the structural-vs-actionable distinction explicit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cover the untested gh-token-utils module (token resolution branches, env var fallback, no-token path) and the repo-sync-runner orchestration (successful sync, error isolation, CLI options merging, dry-run lifecycle). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…iven loop in unified-summary The formatCombinedSummary function repeated the same formatCountEntry+selectLabel pattern for each settings entity type. This replaces those 3 blocks with a settingsSummaryDescriptors array and a loop, consistent with the descriptor pattern used in settings-runner.ts. Also adds the missing variables entity type to the summary and change-detection checks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ctors in settings-report-builder The buildSettingsReport function had 3 near-identical collect-filter-count-accumulate blocks for rulesets, labels, and variables. Replaced with a data-driven EntityCollector pattern using makeEntityCollector factory, preserving full type safety. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…-runner, xfg-template, github-lifecycle-provider - settings-report: replace 4 repeated formatCountEntry blocks with descriptor-driven loop - repo-sync-runner: rename pushFailure to recordFailure to avoid git push naming collision - xfg-template: replace verbose 20-line JSDoc with concise description and @see reference - github-lifecycle-provider: extract duplicated inline log adapter to shared retryLog variable Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…plication Move settingsSummaryDescriptors and action label mappings to a shared module imported by both settings-report.ts and unified-summary.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract duplicated plan summary-line generation from labels and variables formatters into a shared formatPlanSummary() in base-processor.ts. Add retryable field to PRResult so callers can distinguish permanent vs transient PR creation failures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tories Organize the flat 16-file sync module into concern-based subdirectories following the same pattern used for vcs/ restructure. Each subdirectory gets a barrel index.ts, and the root barrel re-exports everything for backward compatibility. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The alias `const result = accumulated` caused delete operations to mutate the caller's object through the reference. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace imported SettingsReport type with inline string literal union to eliminate circular dependency between settings-report.ts and settings-summary-descriptors.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…s formatters Eliminate near-identical group-by-action scaffolding between labels and variables formatters by extracting a generic helper with callbacks for action-specific rendering. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SecretsProcessor now implements ISettingsProcessor like all other settings processors, using withGitHubGuards, buildDryRunResult/buildApplyResult, ChangeCounts, and the standard process(repoConfig, repoInfo, options) signature. The global SecretsConfig is now injected via the constructor since secrets are config-level (not per-repo), while the method signature matches the interface. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Align with all other settings processors that use the shared result builder for consistency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Classify changes once before the dry-run/apply branch instead of duplicating the counting logic in both branches. Remove dead comments about unchanged counter semantics. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merge Protections🔴 1 of 2 protections blocking · waiting on 👀 reviews
🔴 defaultWaiting for
This rule is failing.
Show 1 satisfied protection🟢 🚦 Auto-queueWhen all merge protections are satisfied, this pull request will be queued automatically. |
- secrets-command.ts: throw SyncError instead of Error - env-resolver.ts: throw ValidationError instead of Error Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
- Remove 2 restating section comments from pr-creator.ts - Replace manual strategy enumeration with arrayMergeStrategies.has() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…cts and secrets diff Move src/secrets/ to src/settings/secrets/ to colocate with other settings processors. Add VariableCreateParams/VariableUpdateParams interfaces to IVariablesStrategy for consistent params-object signatures. Extract secrets diff logic into a dedicated diffSecrets module mirroring variables/diff.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add settings/secrets/, vcs/pr|commit|auth/, sync/file|manifest|diff/ barrel and type files to the ignore list after directory restructuring. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add `/* c8 ignore next */` to unreachable exhaustive switch defaults in labels/processor, rulesets/processor, variables/processor, file-status, diff-utils, merge (mergeTextContent), and encryption (import failure). Exclude type-only files (secrets/types.ts, variables/types.ts) from c8 coverage. Add secrets-command tests for success:false result, non-GitHub repo token resolution, and retries default. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
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
2 participants
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.
Summary
Comprehensive code health improvements driven by desloppify analysis. Strict score improved from 87.0 → 92.4 (+5.4 points) across 27 commits touching 96 files.
Key changes:
src/vcs/intopr/,commit/,auth/subdirectories;src/sync/intofile/,manifest/,diff/ISettingsProcessorwithwithGitHubGuards,buildDryRunResult/buildApplyResult,ChangeCountsRecord<string,X> & { meta?: boolean }intersection types with proper named interfaces (VariablesConfig,SecretsConfig,RawVariablesMap, etc.)validateGitHubResourceName,checkVariableSecretOverlap,computeContentPresenceFlagsshared helpersmergeVariablesWithMetahelper, fixedapplyFileLayershallow copyformatGroupedPlanshared helper,formatPlanSummaryshared, descriptor-driven summaries,EntityCollectorpatternget*→build*in PR strategies,pushFailure→recordFailureretryabletoPRResult, consistentresolveGitHubTokenin secrets commandsync-utils,gh-token-utils,repo-sync-runnerTest plan
npm run buildpassesnpm test— 2987 tests, 0 failuresnpm run test:typecheckpasses./lint.shpasses (pre-existing gitleaks false positives only)🤖 Generated with Claude Code