Repository navigation
refactor: CommitErrors to use Vec instead of HashMap - #507
Merged
Merged
Conversation
`Commits::lint` cloned the entire commit list twice: once by iterating `self.commits.iter().cloned()` (cloning clean commits that were then discarded), and again via `self.commits.clone()` to give `CommitErrors` an ordering for its `HashMap`. Collect `Vec<(Commit, Vec<CommitError>)>` in walk order instead. Only commits that actually have errors are cloned, the `order` field and the `HashMap` are gone, and both formatters iterate the pairs directly rather than looking each commit up. `Commit` no longer needs to derive `Hash` over its full message. Output is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ssytzRh2x7PPvEJEndD4t
DeveloperC286
changed the title
Both result values were built in two steps: a bare collection, then a second `let` that shadowed it to wrap the value in its error type. Fold each into its producer. The per-commit loop moves inside a block whose tail expression is the `Option<CommitErrors>`, and the aggregate check becomes `Option::filter` + `map` constructing `CommitsErrors` directly, so the `and_then` with its `Some`/`None` arms is gone. Hoisting `actual_commits` lets both struct fields use field-init shorthand. Output is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ssytzRh2x7PPvEJEndD4t
Replace the accumulate-into-a-mutable-Vec loop with a `filter_map` over the commits, and wrap the result with the same `filter`/`map` shape the aggregate errors already use, so both halves of `lint` read alike and neither needs a block or a shadowing `let`. `collect` cannot express "None when nothing matched", so the emptiness test stays; `Some(..).filter(..)` keeps it inside the one chain. `CommitError` is no longer named here, so drop its import. Output is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ssytzRh2x7PPvEJEndD4t
`CommitErrors::new` and `CommitsErrors::new` were infallible, so `lint()` had to work out for each one whether to call it at all — filtering an empty Vec for the former, comparing max_commits against actual_commits for the latter — before wrapping the result in Some/None itself. Give each constructor the raw inputs and let it return `Option<Self>`: `CommitErrors::new` takes the collected Vec and is `None` when it's empty; `CommitsErrors::new` takes `max_commits` and `actual_commits` and is `None` unless the threshold is exceeded. Both invariants now live on the type that owns them instead of at the one call site, and `lint()` calls each constructor directly with no filter/map/then needed to get an `Option` out of it. Output is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ssytzRh2x7PPvEJEndD4t
`CommitsErrors::new` was doing the max_commits/actual_commits threshold
comparison itself, which put business logic inside the type unlike
`CommitErrors::new`'s plain emptiness check. Give it the same shape:
take a `Vec<CommitsError>` and be `None` only when it's empty.
The threshold decision moves back to `lint()`, which now builds a
zero-or-one-element Vec via `Option::filter`/`map`/`into_iter().collect()`
before handing it to the constructor. Both constructors are now
identical in form: `(!errors.is_empty()).then_some(Self { errors })`.
Output is unchanged.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ssytzRh2x7PPvEJEndD4t
The `match (commit_errors, commits_errors) { (None, None) => None, ... }`
at the end of `lint()` was the same "does this exist" decision as
`CommitErrors::new` and `CommitsErrors::new` already make for
themselves, just spelled out with a match instead of delegated.
Add `LintingResults::new`, following the same
`<condition>.then_some(Self { .. })` shape as the other two
constructors: `is_some() || is_some()` stands in for `!is_empty()`
since there's no Vec here, only two Options. `lint()` now ends with a
single call to it instead of the match block.
Output is unchanged.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ssytzRh2x7PPvEJEndD4t
…bility Two spots read worse than what they replaced: - `LintingResults::new` used `(is_some() || is_some()).then_some(..)`. `then_some` takes its value eagerly, so the struct is always built even on the `None` path — a reader has to know that to be sure nothing wasteful happens. The match makes the two cases, and that nothing is built in the `(None, None)` arm, explicit. - The aggregate-errors vec was built via `max_commits.filter(..).map(..).into_iter().collect()`, leaning on the "Option collected into a Vec is zero-or-one elements" idiom. A match on `max_commits` with a guard says the same thing directly, without requiring that idiom to parse it. Output is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ssytzRh2x7PPvEJEndD4t
DeveloperC286
enabled auto-merge (squash)
September 8, 2026 17:34
DeveloperC286
deleted the
claude/lint-commit-clone-duplication-szwhhn
branch
September 8, 2026 17:36
Merged
DeveloperC286
added a commit
that referenced
this pull request
Sep 8, 2026
🤖 I have created a release *beep* *boop* --- ## 1.2.1 (2026-09-08) ## What's Changed * chore(deps): update dependency https://github.com/developerc286/template to v1.7.5 by @DeveloperC286 in #449 * chore(deps): update rust crate clap to v4.5.60 by @renovate[bot] in #451 * chore(deps): update rust crate anyhow to v1.0.102 by @renovate[bot] in #452 * chore(deps): update nix flake lock by @renovate[bot] in #454 * chore(deps): update dependency https://github.com/developerc286/template to v1.7.6 by @renovate[bot] in #456 * chore(deps): update nix flake lock by @renovate[bot] in #455 * chore(deps): update rust crate clap to v4.5.61 by @renovate[bot] in #457 * chore(deps): update rust crate clap to v4.6.0 by @renovate[bot] in #458 * chore(deps): update nix flake lock by @renovate[bot] in #459 * chore(deps): update nix flake lock by @renovate[bot] in #460 * chore(deps): update nix flake lock by @renovate[bot] in #461 * chore(deps): update nix flake lock by @renovate[bot] in #462 * chore(deps): update nix flake lock by @renovate[bot] in #463 * chore(deps): update dependency https://github.com/developerc286/template to v1.7.7 by @renovate[bot] in #464 * chore(deps): update alpine docker tag to v3.23.4 by @renovate[bot] in #465 * chore(deps): update rust crate clap to v4.6.1 by @renovate[bot] in #466 * chore(deps): update alpine:3.23.4 docker digest to 5b10f43 by @renovate[bot] in #467 * chore(deps): update nix flake lock by @renovate[bot] in #468 * chore(deps): update nix flake lock by @renovate[bot] in #469 * chore(deps): update dependency https://github.com/developerc286/template to v1.7.8 by @renovate[bot] in #470 * chore(deps): update nix flake lock by @renovate[bot] in #471 * chore(deps): update nix flake lock by @renovate[bot] in #473 * chore(deps): update nix flake lock by @renovate[bot] in #474 * chore(deps): update nix flake lock by @renovate[bot] in #476 * chore(deps): update rust crate log to v0.4.30 by @renovate[bot] in #477 * chore(deps): update rust crate log to v0.4.32 by @renovate[bot] in #478 * chore(deps): update nix flake lock by @renovate[bot] in #479 * chore(deps): update nix flake lock by @renovate[bot] in #481 * chore(deps): update rust crate log to v0.4.33 by @renovate[bot] in #482 * fix(deps): update rust crate git2 to 0.21.0 by @renovate[bot] in #475 * chore(deps): update rust crate anyhow to v1.0.103 by @renovate[bot] in #483 * chore(deps): update alpine docker tag to v3.24.1 by @renovate[bot] in #480 * chore(deps): update dependency https://github.com/developerc286/template to v1.7.9 by @renovate[bot] in #484 * chore(deps): update rust crate clap to v4.6.2 by @renovate[bot] in #485 * chore(deps): update rust crate anyhow to v1.0.104 by @renovate[bot] in #486 * fix: reject 0 as an invalid --max-commits value by @DeveloperC286 in #487 * chore(deps): update rust crate clap to v4.6.3 by @renovate[bot] in #488 * refactor: build result fields independently by @DeveloperC286 in #489 * chore(deps): update dependency https://github.com/developerc286/template to v1.7.10 by @renovate[bot] in #490 * refactor: use consistent short commit hash across output formats by @DeveloperC286 in #491 * refactor: set log level explicitly instead of mutating RUST_LOG by @DeveloperC286 in #492 * refactor: log macro imports to edition 2021 style by @DeveloperC286 in #493 * docs: align Cargo.toml description with README pitch by @DeveloperC286 in #494 * build: update to Rust edition 2024 by @DeveloperC286 in #495 * chore(deps): update rust crate clap to v4.6.4 by @renovate[bot] in #496 * chore(deps): update nix flake lock by @renovate[bot] in #497 * chore(deps): update rust crate clap to v4.6.5 by @renovate[bot] in #498 * chore(deps): update rust crate clap to v4.6.6 by @renovate[bot] in #500 * chore(deps): update nix flake lock by @renovate[bot] in #499 * chore(deps): update dependency https://github.com/developerc286/template to v1.7.11 by @renovate[bot] in #501 * chore(deps): update nix flake lock by @renovate[bot] in #502 * chore(deps): update rust crate log to v0.4.34 by @renovate[bot] in #503 * chore(deps): update nix flake lock by @renovate[bot] in #504 * chore(deps): update nix flake lock by @renovate[bot] in #505 * chore(deps): update nix flake lock by @renovate[bot] in #506 * refactor: CommitErrors to use Vec instead of HashMap by @DeveloperC286 in #507 **Full Changelog**: v1.2.0...v1.2.1 --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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.
No description provided.