Skip to content

Conversation

@candiduslynx
Copy link
Contributor

No description provided.

@candiduslynx candiduslynx force-pushed the feat/jsonschema/configtype/duration branch from 2ea2141 to 8b6ef58 Compare October 17, 2023 09:59
@github-actions
Copy link

github-actions bot commented Oct 17, 2023

⏱️ Benchmark results

  • Glob-8 ns/op: 91.09

@candiduslynx candiduslynx changed the title feat: add JSON schema for configtype.Duration feat: Add JSON schema for configtype.Duration Oct 17, 2023
@github-actions github-actions bot added feat and removed feat labels Oct 17, 2023
Spec: `false`,
},
},
func() []testCase {
Copy link
Member

@hermanschaaf hermanschaaf Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this syntax of appending a function that calls itself and appends the results in-line feels unnecessarily complicated, it'd be more readable to introduce a variable first and append that, like:

generatedCases := make([]testcase, 20)
for i := 0; i < 20; i++ {
     generatedCases[i] = generateTestCase()
}

...
cases := append([]testCase{
		{
			Name: "empty",
			Err:  true,
			Spec: `""`,
		},
                ...,
                generatedCases...
})

or have a few manually created test cases, there doesn't seem to be enough complexity here to warrant generating random cases

@kodiakhq kodiakhq bot merged commit 5e1598b into main Oct 17, 2023
@kodiakhq kodiakhq bot deleted the feat/jsonschema/configtype/duration branch October 17, 2023 10:32
kodiakhq bot pushed a commit that referenced this pull request Oct 17, 2023
🤖 I have created a release *beep* *boop*
---


## [4.15.0](v4.14.1...v4.15.0) (2023-10-17)


### Features

* Add JSON schema for `configtype.Duration` ([#1303](#1303)) ([5e1598b](5e1598b))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants