Repository navigation
Mark win* GODEBUG settings as Windows-only (#3) - #7
Conversation
winsymlink and winreadlinkvolume only change behavior on Windows. Text and JSON reports now say so, so Linux-only services can skip them. Fixes GillMacedo#3
GillMacedo
left a comment
There was a problem hiding this comment.
The feature works, but two things need changing before this can merge.
1. The saved example output is out of date. The repo's CLAUDE.md says example output is real output and must be regenerated when the format changes, and README.md links examples/shop-1.21-to-1.24.txt as the full output. To reproduce:
go run ./plugin/scripts --from 1.21 --to 1.24 examples/shop | diff examples/shop-1.21-to-1.24.txt -
Lines 20 and 25 differ, because the saved file lacks (Windows only). Please run make examples and commit the result. examples/real/cobra.txt also lists these settings, so update it too (it needs the project cloned, see examples/README.md).
2. No test checks that compare() sets the flag. TestWindowsOnly builds its Change with WindowsOnly: true by hand. If you delete c.WindowsOnly = isWindowsOnly(k) from compare(), go test -count=1 ./... still passes, yet real reports and JSON lose the mark. Please add a test that goes through compare() (or the integration tests in tests/) and asserts that winsymlink gets windowsOnly: true and a non-win setting doesn't.
What passed: make test. On examples/shop from 1.22 to 1.23, the text report marks winsymlink and winreadlinkvolume as (Windows only), and JSON has "windowsOnly": true only for those two. A bad --from value and a missing directory both give a clear error and exit 2. Go 1.27.3's settings table has only these two win settings, so the prefix check the issue asks for is correct today.
GillMacedo
left a comment
There was a problem hiding this comment.
Both changes from the earlier review at this commit (fde6993) are still needed. I checked them again and they still reproduce.
1. The saved example output is out of date. The repo CLAUDE.md says example output is real output and must be regenerated when the format changes. To reproduce:
go run ./plugin/scripts --from 1.21 --to 1.24 examples/shop | diff examples/shop-1.21-to-1.24.txt -
Lines 20 and 25 differ, because the saved file lacks (Windows only). Run make examples and commit the result. examples/real/cobra.txt lines 15 and 20 list the same settings without the mark, so update that file too (it needs the project cloned, see examples/README.md).
2. No test checks that compare() sets the flag. TestWindowsOnly builds its Change with WindowsOnly: true by hand. To reproduce, delete c.WindowsOnly = isWindowsOnly(k) from compare() and run go test -count=1 ./...: both packages still pass, yet real reports and JSON would lose the mark. Add a test that goes through compare() or the integration tests in tests/, and asserts that winsymlink gets windowsOnly: true and a non-win setting does not.
What passed: make test, with no owner paths or risky lines. On examples/shop from 1.22 to 1.23, the text report marks winsymlink and winreadlinkvolume as (Windows only), and JSON has "windowsOnly": true only for those two. --from nope gives a clear error and exit status 2. The code does what issue #3 asks.
GillMacedo
left a comment
There was a problem hiding this comment.
The code does what issue #3 asks. Checked: make test passes, with no owner paths or risky lines. On examples/shop from 1.22 to 1.23, the text report marks only winsymlink and winreadlinkvolume as (Windows only), and JSON has "windowsOnly": true only for those two. --from nope gives a clear error and exit status 2. Go 1.27.3 has only these two win settings. The stale saved examples and the missing test through compare() are small under the current rules and are tracked in #11, which replaces the earlier change requests.
Summary
Settings whose names start with
win(e.g.winsymlink,winreadlinkvolume) only change behavior on Windows. Text reports now append(Windows only), and JSON includes"windowsOnly": true, so Linux-only services can skip them.Test plan
make test(gofmt, vet, unit + integration)TestWindowsOnlyfails without the mark and passes with itFixes #3
AI disclosure: This PR was authored with assistance from an AI coding agent (Cursor). I reviewed the change and ran
make test.