Skip to content

fix: expose launch set name args directly#29

Merged
wizzomafizzo merged 1 commit into
mainfrom
fix/launch-set-name-decoding
May 26, 2026
Merged

fix: expose launch set name args directly#29
wizzomafizzo merged 1 commit into
mainfrom
fix/launch-set-name-decoding

Conversation

@wizzomafizzo
Copy link
Copy Markdown
Member

@wizzomafizzo wizzomafizzo commented May 26, 2026

Summary

  • move set_name and set_name_same_dir onto each launch arg struct directly
  • keep key constants from v0.9.0
  • add reflection coverage that launch arg structs expose the advarg tags

Test

  • go test ./...

Summary by CodeRabbit

  • Refactor

    • Reorganized internal argument structure to improve code maintainability and reduce duplication.
  • Tests

    • Updated test suite to reflect code restructuring.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The PR removes the LaunchSetNameArgs embedded type and inlines its SetName and SetNameSameDir fields directly into five launch-related argument structs. A new reflection-based test validates the advarg tags are present, and an existing test is updated to use the new field structure.

Changes

SetName field inlining refactoring

Layer / File(s) Summary
Inline SetName fields into launch argument types
types.go
LaunchArgs, LaunchRandomArgs, LaunchSearchArgs, LaunchTitleArgs, and LaunchLastArgs each remove the LaunchSetNameArgs embedding and declare SetName and SetNameSameDir fields directly with matching advarg tags.
Advanced argument field validation test
types_advargs_test.go
New test file introduces TestLaunchArgsSetNameAdvargFields and a reflection-based requireAdvargField helper that verifies set_name and set_name_same_dir advarg tags are present across all five launch argument types.
Update existing test for new field structure
types_test.go
TestLaunchArgsSetNameFields is updated to initialize SetName and SetNameSameDir directly on the LaunchArgs struct literal instead of nesting them under a LaunchSetNameArgs field.

Possibly Related PRs

  • ZaparooProject/go-zapscript#28: Adds the same set_name / set_name_same_dir fields to launch argument structs via LaunchSetNameArgs embedding, which this PR refactors to direct field inlining.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Five structs stood tall with fields embedded, now they hold their own, unforgetted. A test with mirrors (and reflection's grace) checks that each advarg tag stays in place. No embedding here—just inline fields, inlined with care—simplicity yields! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: expose launch set name args directly' accurately describes the main change: inlining SetName/SetNameSameDir fields directly into launch argument structs instead of embedding LaunchSetNameArgs.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/launch-set-name-decoding

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@wizzomafizzo wizzomafizzo merged commit 63a13b1 into main May 26, 2026
11 of 12 checks passed
@wizzomafizzo wizzomafizzo deleted the fix/launch-set-name-decoding branch May 26, 2026 08:08
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.

1 participant