[#19] Change token symbol prefix PLT- → PL-#22
Conversation
Update StoryFactory to produce PL-1, PL-2, etc. for mainnet to avoid collision with old testnet contracts. Fixes #19 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
T2b APPROVE
- One-line change:
PLT-→PL-inStoryFactory.sol:114— matches issue #19 spec - No other references to
PLTin the repo (MeasureGas.s.sol doesn't check symbol output) - All 15 Foundry tests pass, CI green
- Clean, minimal change. LGTM.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The production change itself is the expected one-line prefix update, and CI/local forge test are green. The review blocker is that the PR still does not verify the core acceptance criterion: that createStoryline() actually passes PL-<id> into token creation.
Findings
- [medium] No test asserts the generated token symbol after changing the prefix.
- File:
test/StoryFactory.t.sol - Suggestion: update the mock bond to capture
TokenParamsand add an assertion thatcreateStoryline()sendsPL-1,PL-2, etc. This is the behavior the issue is specifically changing, and right now all 15 tests can pass while that value remains unverified.
- File:
Decision
Request changes until the symbol output is covered. The implementation is likely correct, but the PR does not yet prove the acceptance criterion it is supposed to satisfy.
Capture TokenParams in MockBond and assert createStoryline produces PL-1, PL-2 symbols. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up commit closes the only review gap by asserting the generated symbol directly in tests. The implementation remains minimal and now the acceptance criterion is proven.
Findings
- [info]
test_createStoryline_symbolPrefixcapturesTokenParams.symbolin the mock bond and verifiesPL-1/PL-2.
Decision
Approve because the PR now both implements and verifies the required prefix change.
Summary
PLT-toPL-inStoryFactory.solline 114PL-1,PL-2, etc. for mainnetTest plan
forge test— all 15 tests passforge fmt— cleancreateStoryline()producesPL-<id>symbols on deployFixes #19
🤖 Generated with Claude Code