Skip to content

Simplify pass + code-review fixes on today's changes - #374

Merged
philoserf merged 1 commit into
mainfrom
simplify-and-review-fixes
Aug 5, 2026
Merged

Simplify pass + code-review fixes on today's changes#374
philoserf merged 1 commit into
mainfrom
simplify-and-review-fixes

Conversation

@philoserf

Copy link
Copy Markdown
Owner

Summary

  • Removed dice.Fixed (unused speculative scaffolding — flagged by all three cleanup review angles as having zero real callers) and its tests.
  • Collapsed shipgen.structureData's aliases []string field to a scalar alias string, since only one row ever used it.
  • Merged each golden-fixture / README-freshness test pair in cmd/{worldgen,chargen,sectorgen,shipgen}/golden_test.go into a single test with t.Run subtests, so each pair shares one subprocess spawn instead of two.
  • cmd/worldgen/main.go: -format json now coerces a nil TradeCodes slice to [] before encoding (was rendering null for a world matching no trade code).
  • cmd/worldgen/main.go: flush the buffered stdout writer before a mid-loop cli.Fatalf, so an encode error no longer silently discards already-generated records.
  • internal/shipgen/design_test.go: TestDesignMurphy's core-cost assertion now pins Fuel.Cost independently (matching TestFuelMurphy's own formula) instead of reading it back off the struct under test, so it can actually catch a fuel-cost regression.

Test plan

  • task check (golangci-lint + full test suite) green

Removed dice.Fixed (unused speculative scaffolding), collapsed shipgen's
structureData aliases []string to a scalar, and merged golden/README-freshness
test pairs to share one subprocess run each. Also fixed two worldgen
-format json bugs found by review: nil TradeCodes rendering as null instead
of [], and a mid-loop Fatalf silently dropping already-buffered stdout
records. Made shipgen's core-cost test assertion non-tautological by pinning
Fuel.Cost independently instead of reading it back off the struct under test.

Co-Authored-By: Claude
Copilot AI lite review requested due to automatic review settings August 5, 2026 22:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR continues the repo’s ongoing cleanup/reproducibility work by removing unused dice scaffolding, simplifying a shipgen lookup table, tightening one shipgen regression test, and reducing CLI golden-test overhead while fixing a couple of worldgen JSON/IO edge behaviors.

Changes:

  • Remove unused dice.Fixed and its tests.
  • Simplify shipgen structure alias handling (slice → single alias) and update StructureByName accordingly.
  • Improve CLI stability/perf: worldgen JSON tradeCodes now renders [] instead of null for no-code worlds; golden/README tests share a single subprocess run via subtests.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/shipgen/shipgen.go Collapses structure alias data to a single optional alias string.
internal/shipgen/generate.go Updates StructureByName to match against the single alias field.
internal/shipgen/design_test.go Makes the Murphy core-cost assertion non-tautological by pinning fuel cost independently.
internal/dice/dice.go Removes the unused Fixed roller constructor.
internal/dice/dice_test.go Removes tests specific to the deleted Fixed helper.
cmd/worldgen/main.go JSON output now uses [] for empty trade codes and flushes buffered stdout before fatal exit on encode error.
cmd/worldgen/golden_test.go Merges golden + README freshness checks into subtests sharing one run.
cmd/shipgen/golden_test.go Merges golden + README freshness checks into subtests sharing one run.
cmd/sectorgen/golden_test.go Merges golden + README freshness checks into subtests sharing one run (with README prefix comparison).
cmd/chargen/golden_test.go Merges golden + README freshness checks into subtests sharing one run.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cmd/worldgen/main.go
@philoserf
philoserf merged commit 5ba4e8e into main Aug 5, 2026
2 checks passed
@philoserf
philoserf deleted the simplify-and-review-fixes branch August 5, 2026 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants