Add support for Devices Filtering by Fields#75
Add support for Devices Filtering by Fields#75nikita-vanyasin wants to merge 3 commits intotailscale:mainfrom
Conversation
| server.ResponseCode = http.StatusOK | ||
| server.ResponseBody = expectedDevices | ||
|
|
||
| t.Run("with single filter", func(t *testing.T) { |
There was a problem hiding this comment.
nit: we try to avoid any characters in test subnames that result in quoting by the testing package. So no spaces.
And we aim for brevity in the test subname, expanding on the brevity if necessary in a comment.
So this could be "single-filter" or "single"
| assert.EqualValues(t, expectedDevices["devices"], actualDevices) | ||
| }) | ||
|
|
||
| t.Run("with multiple filters", func(t *testing.T) { |
There was a problem hiding this comment.
"multiple-filters" or even "multiple"
| assert.EqualValues(t, expectedDevices["devices"], actualDevices) | ||
| }) | ||
|
|
||
| t.Run("with all fields and filter", func(t *testing.T) { |
| assert.EqualValues(t, expectedDevices["devices"], actualDevices) | ||
| }) | ||
|
|
||
| t.Run("with empty options", func(t *testing.T) { |
|
Hi @bradfitz and thanks for review. Best, |
|
@nikita-vanyasin Thank you for the contribution! We’d like to merge it. But following an internal discussion and we’d actually like to take it one step further. We want to turn the options argument approach you’ve sketched out into a full-blown options pattern and make that the new default for this endpoint (and potentially other similar endpoints in the future for the next major release). What I’ll do is I’ll take your commits and add that additional change on top in a separate PR. |
|
Closing this as we've merged this contribution into #82. |
This is great, thank you! |
4 participants
FYI this PR is mostly vibe-coded.