refactor: unify walkers and breakdown printers, dedup shared helpers#4
Merged
Conversation
Consolidate the two near-identical projects-directory walkers into a single dirWalker (walker.go) driven by a per-file callback. Extract localMidnightCutoff and hasJSONType helpers to remove the cutoff and "type" byte-check duplication across parser.go and tools.go, and add newParseResult/newToolResult constructors for the repeated map init. Collapse the daily/monthly and project/branch breakdown printers in format.go, and remove the formatFiveHourUsage and formatStatusline shims that existed only for tests (tests now call the real functions).
Skill-listing lines without a ": description" suffix (e.g. "- optimize-tests") were skipped entirely, so those skills never appeared in the available-skills set and were misreported as unused. Treat the trimmed line as the skill name when no separator is present.
Route all 13 sort-and-truncate sites in format.go through one generic helper instead of hand-rolling sort.Slice + topN guards at each call.
backstabslash
force-pushed
the
code-review-cleanup
branch
from
1e3ba3b to
6ac509b
Compare
backstabslash
changed the title
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.
Summary
Maintainability pass from a full-repo code review, plus one behavioral fix. Three commits, reviewable independently.
refactor: unify log/tool walkers and drop test-only shimsdirWalker(walker.go) driven by a per-file callback.localMidnightCutoffandhasJSONTypehelpers, removing the cutoff and"type"byte-check duplication acrossparser.go/tools.go.newParseResult/newToolResultconstructors for the repeated map initialization.formatFiveHourUsageandformatStatuslineshims that existed only for tests; tests now call the real functions.fix: count skills listed without a description in tool analyticsSkill-listing lines without a
": description"suffix (e.g.- optimize-tests) were skipped, so those skills never entered the available-skills set and were misreported as unused. The trimmed line is now used as the skill name when no separator is present.refactor: collapse repeated sort+topN logic into generic sortAndTrimRoute the ~13 sort-and-truncate sites in
format.gothrough one genericsortAndTrimhelper instead of hand-rollingsort.Slice+ topN guards at each call. Also consolidates the daily/monthly and project/branch breakdown printers (printPeriodBreakdown,printBreakdownGroup) and the JSON daily/monthly rows (jsonPeriodRow).Verification
make checkgreen (vet, gofmt,go test -race, golangci-lint, gocyclo). New tests cover the breakdown print paths (previously only exercised via the removed shims), bare skill-name parsing, and the JSON daily/monthly builders.