refactor: remove duplication found in code review - #118
Open
tylerbutler wants to merge 10 commits into
Open
Conversation
Route every git invocation through git_output; delete the private git_stdout copy in release.rs; collapse tag.rs rev_parse/remote_tag_oid onto local_tag_oid/remote_tag_oids. Claude-Session: https://claude.ai/code/session_015R9SdGkqtNm1mTzyCR9kZZ
The three variants carried identical fields; four accessor matches existed only to re-extract them. Also drop the clone into in_package and collapse check_member_glob to one push. Claude-Session: https://claude.ai/code/session_015R9SdGkqtNm1mTzyCR9kZZ
…mber patch_locked_versions inlined the two-shape walk twice; both patchers now share patch_packages. set_str_keep_decor replaces five copies of the replace-but-keep-decor block. doctor and version apply share patch_member. Claude-Session: https://claude.ai/code/session_015R9SdGkqtNm1mTzyCR9kZZ
Invalid exclusion globs were reported twice (once by the exclude sweep, again for @Release and @members). ConfigFile::from_document takes the already-parsed root manifest. compile_overrides and Diagnostics config_error/config_warning replace repeated blocks; Member::rel_file, has_series_tag and Workspace::series_tags_of replace four copies of the series-tag iteration. Claude-Session: https://claude.ai/code/session_015R9SdGkqtNm1mTzyCR9kZZ
Move fixture, trellis, git, json_output and the other duplicated helpers into tests/common/mod.rs; every test file now imports them. Inline git init/add/commit triples use init_repo and JSON-parsing blocks use json_output. 413 tests, snapshots unchanged. Claude-Session: https://claude.ai/code/session_015R9SdGkqtNm1mTzyCR9kZZ
phase2 -> changelog_version, phase3 -> tag_publish, new_and_release -> release_pr; member_discovery merged into configless; doctor and publish tests moved next to their peers; the two invalid-fragment tests merged into one; seven cli.rs --json tests removed because the json_contract snapshots already pin every assertion. Tests: 413 before, 405 after. Claude-Session: https://claude.ai/code/session_015R9SdGkqtNm1mTzyCR9kZZ
- runner: RunOptions::parallelism and run_and_report shared by run/exec; channel carries (job_idx, JobOutcome); run_jobs takes &[Job]; one paint_status; name width computed once - github: url() and expect_status() helpers, Write enum for POST/PATCH, read_response returns the tuple, iterator-based token lookup - pin: one status line and insert per rewrite arm; entry API in resolve - lockfile: use Workspace::select for the single-package case - tools: with_retry loops 1..attempts then makes the final call directly - info: or_none helper; generate: write completions to a locked stdout Claude-Session: https://claude.ai/code/session_015R9SdGkqtNm1mTzyCR9kZZ
…on, changelog - main: fold doctor into the first dispatch match; call tag::create for bootstrap - git: add git_with_identity for commit/tag calls that need the CI identity - tag: claim closure in plan_tags, Copy CreateOptions with push normalized once, release_notes returns Option and is shared with the PR body - changelog: heading_version shared with tag, one minijinja Environment per section, callers pass &Member instead of re-resolving member_index - version: PlanEntry holds semver::Version and its member index - changelog check: one EntryState enum drives text and preview output; Strictness::key replaces the hand-rolled match Claude-Session: https://claude.ai/code/session_015R9SdGkqtNm1mTzyCR9kZZ
Redundant closures, map_or, raw-string hashes, merged identical match arms, literal separators, and an unused self argument. One-time sweep; pedantic is not added to CI. Claude-Session: https://claude.ai/code/session_015R9SdGkqtNm1mTzyCR9kZZ
✅ Deploy Preview for trellis-gleam canceled.
|
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
Whole-repo cleanup from a review for duplicate code, simplifiable structures, and idiom. No behavior changes intended; CLI output, JSON, and error text are unchanged except where noted below.
git_outputhelper behind every git spawn; duplicategit_stdoutin release.rs deleted;repo_root/slash_pathreused;remote_tag_oidcollapses ontoremote_tag_oids.Fixis a struct with akindinstead of three identical variants.set_str_keep_decor,patch_membershared by doctor andversion apply.ConfigFile::from_document;compile_overrides,Diagnostics::config_error,Member::rel_file/has_series_tag,Workspace::series_tags_of.tests/common/mod.rs; files renamed by subject (changelog_version.rs,tag_publish.rs,release_pr.rs);member_discovery.rsmerged intoconfigless.rs; 7--jsonshape tests already pinned by snapshots removed, 2 invalid-fragment tests merged.run_and_reporttail; job index no longer smuggled throughJobResult; GitHub URL and status-check helpers; pin arms hoisted;with_retrysimplified.dispatchmatches once;bootstrapdeleted;git_with_identity;PlanEntryholdssemver::Versionand member index;heading_versionandrelease_notesshared;Strictness::key; minijinjarender_str.Deferred items are marked with
ponytail:comments.Behavior notes
lockfile refreshunknown-package error now lists members (goes throughworkspace.selectlike other commands).Bump.current/nextin the JSON document are owned strings now; wire output identical.