Skip to content

chore: use same tag filtering for ENVs#1014

Merged
mbevc1 merged 3 commits into
mainfrom
20260710_list_envs
Jul 10, 2026
Merged

chore: use same tag filtering for ENVs#1014
mbevc1 merged 3 commits into
mainfrom
20260710_list_envs

Conversation

@mbevc1

@mbevc1 mbevc1 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Let's use same tag filetrs as we use for other commands, like Controls and Repos. This change also allows having commas in tag filters.

Also add missing update control test

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mbevc1's task in 1m 26s —— View job


Review: chore: use same tag filtering for ENVs

  • Gather context (diff, related commands)
  • Static analysis (go build, go vet — both clean)
  • Review code changes
  • Post review

Verdict: LGTM. Small, well-scoped change that correctly aligns list environments with list controls and list repos.

What the change does

  • cmd/kosli/listEnvironments.go:110 — switches the --tag flag from StringSliceVar to StringArrayVar. This is the substantive fix: StringSliceVar splits each value on commas, so a tag like key:a,b would be silently broken into two filters. StringArrayVar preserves the whole value and only accumulates one entry per --tag occurrence — matching listControls.go:98 and listRepos.go:106. ✅
  • This composes correctly with the query builder at listEnvironments.go:138-140, which does params.Add("tag", tag) per element, so each --tag becomes its own tag= query param. ✅
  • cmd/kosli/root.go:319 — help text reworded to match the controls/repos wording ("Can be repeated to match more than one tag"). Consistent. ✅
  • cmd/kosli/lifecycle_test.go:48 — adds the missing update control entry to the beta-commands test. Good catch; closes a gap where newUpdateControlCmd wasn't asserted as beta. ✅

Verification

  • go build ./... — passes
  • go vet ./cmd/kosli/ — passes

Minor / optional

  • The PR notes the change "allows having commas in tag filters," but there's no test exercising that behavior directly for list environments (understandable, since it needs the local server). If you want to lock it in cheaply, a small unit-style assertion that --tag key:a,b yields a single tag param — rather than two — would prevent a future regression back to StringSliceVar. Not a blocker.

Nice consistency cleanup — this removes a real footgun (comma-splitting) rather than just cosmetic alignment.

@mbevc1 mbevc1 enabled auto-merge (squash) July 10, 2026 11:26
@mbevc1 mbevc1 added the testing label Jul 10, 2026
@mbevc1 mbevc1 merged commit ed098d4 into main Jul 10, 2026
20 checks passed
@mbevc1 mbevc1 deleted the 20260710_list_envs branch July 10, 2026 13:41
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.

2 participants