revert(formula): remove target.version - #187
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: revert target.version auto-property
Clean, coherent revert. The target.version / targetVersion / formulaTarget / .Version() surface is removed completely, with the associated test and testdata dropped and no dangling references left in code or docs.
Verified across the four review dimensions:
- Correctness / dead code: No remaining references to any removed symbol. The
versionparam still threaded throughat()is legitimate (used byfindMaxFromVerfor version resolution) and correctly no longer passed toinjectMatrix. - Documentation: No stale mentions of
target.versionin README/docs/.goxfiles; remaining.Versionhits are the unrelatedmodule.Versionstruct. - Security & performance: No implications — pure removal that doesn't touch caching/locking/cloning hot paths.
Two minor (non-blocking) polish notes inline.
| // In formula DSL, target.require["xxx"] maps to Target().Require()["xxx"]. | ||
| func (t formulaTarget) Require() map[string][]string { | ||
| return t.m.Require | ||
| func (m matrixTarget) Require() map[string][]string { |
There was a problem hiding this comment.
Minor: the renamed matrixTarget methods use receiver name m, which collides with the *Matrix receiver m used elsewhere in this file and yields the awkward m.m.Require / m.m.Options (field is also named m). The pre-revert formulaTarget used t. Consider renaming the receiver back to t for readability and consistency with the surrounding Matrix receivers. Stylistic only — behavior is correct.
| // It is the original version or ref selected by LLAR. | ||
| func (t formulaTarget) Version() string { | ||
| return t.version | ||
| func (p *ModuleF) Target() matrixTarget { |
There was a problem hiding this comment.
Minor: Target() is exported and part of the DSL surface but has no doc comment, while its sibling accessors Require() and Options() each carry a comment referencing their target.* auto-property. A one-line doc comment here would keep the trio consistent now that the returned type was renamed.
7868d84 to
b63d82b
Compare
2 participants
This reverts the Formula-facing
target.versionAPI introduced by #178.The implementation includes:
target.versiongetter fromModuleFtarget.requireandtarget.optionsunchangedgo test -ldflags=-checklinkname=0 ./...This keeps version-dependent Formula behavior represented by explicit
fromVerboundaries instead of hidden branches on raw VCS refs. Closes #186.