Skip to content

move ForeachOptionTypeInLiquid from go-bits to here#82

Merged
wagnerd3 merged 1 commit intomainfrom
liquid-foreachoptiontype
Mar 3, 2026
Merged

move ForeachOptionTypeInLiquid from go-bits to here#82
wagnerd3 merged 1 commit intomainfrom
liquid-foreachoptiontype

Conversation

@majewsky
Copy link
Copy Markdown
Contributor

@majewsky majewsky commented Mar 3, 2026

When we added new relevant option types in 1.20.0, we did not notice the breakage in the ForeachOptionTypeInLiquid test until it hit us in prod. To avoid this going forward, this moves the function into here, so that new relevant types fail loudly and immediately.

This is a straight port of the relevant parts of

https://github.com/sapcc/go-bits/blob/master/liquidapi/liquidapi.go https://github.com/sapcc/go-bits/blob/master/liquidapi/liquidapi_test.go

The only significant change is that the test helper function getOptionTypesRecursively uses go-bits/logg, which is not possible here. I gave it an extra t *testing.T argument and replaced logg.Error with t.Errorf.

When merged, I intend to remove the now-duplicate implementation in go-bits and have it call this implementation.

When we added new relevant option types in 1.20.0, we did not notice the
breakage in the ForeachOptionTypeInLiquid test until it hit us in prod.
To avoid this going forward, this moves the function into here, so that
new relevant types fail loudly and immediately.

This is a straight port of the relevant parts of

<https://github.com/sapcc/go-bits/blob/master/liquidapi/liquidapi.go>
<https://github.com/sapcc/go-bits/blob/master/liquidapi/liquidapi_test.go>

The only significant change is that the test helper function
getOptionTypesRecursively uses go-bits/logg, which is not possible here.
I gave it an extra `t *testing.T` argument and replaced `logg.Error`
with `t.Errorf`.
@majewsky majewsky requested a review from a team as a code owner March 3, 2026 09:30
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2026

Merging this branch will increase overall coverage

Impacted Packages Coverage Δ 🤖
github.com/sapcc/go-api-declarations/liquid 82.65% (+0.05%) 👍

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/sapcc/go-api-declarations/liquid/option.go 100.00% (+100.00%) 5 (+5) 5 (+5) 0 🌟

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/sapcc/go-api-declarations/liquid/option_test.go

@wagnerd3 wagnerd3 merged commit 88153ab into main Mar 3, 2026
6 checks passed
@wagnerd3 wagnerd3 deleted the liquid-foreachoptiontype branch March 3, 2026 10:24
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