Skip to content

Conversation

@ryderhwang
Copy link

@ryderhwang ryderhwang commented Dec 10, 2025

Add --include flag to delete devices command

Summary

This PR adds an --include flag to the tscli delete devices command, allowing users to filter devices by name patterns for inclusion (complementing the existing --exclude flag).

Changes

  • Added --include flag that filters devices by partial name match (can be specified multiple times)
  • Made --include and --exclude mutually exclusive to avoid confusing behavior
  • Added newClient variable for dependency injection (consistent with get/device)
  • Added comprehensive tests for flag validation and helper functions

Usage

Delete only devices matching specific patterns

tscli delete devices --last-seen 24h --include dev --include test --confirm

Using exclude (existing behavior)

tscli delete devices --last-seen 24h --exclude prod --confirm

Cannot use both (returns error)

tscli delete devices --include dev --exclude prod # Error: mutually exclusive### Why mutually exclusive?

Using both --include and --exclude together can lead to confusing semantics (include first then exclude? or vice versa?). Keeping them mutually exclusive makes the behavior predictable and easy to understand.

Testing

go test ./cmd/tscli/delete/devices/... -vAll tests pass, including:

  • Flag validation tests (mutually exclusive check, multiple patterns, etc.)
  • Unit tests for isIncluded and isExcluded helper functions

Important

Add --include flag to tscli delete devices for device name filtering, making it mutually exclusive with --exclude, with tests added.

  • Behavior:
    • Add --include flag to tscli delete devices for filtering devices by name patterns.
    • Make --include and --exclude flags mutually exclusive in cli.go.
  • Functions:
    • Add isIncluded() function in cli.go to check device name inclusion.
    • Modify deleteDisconnectedDevices() in cli.go to handle include logic.
  • Testing:
    • Add TestDeleteDevicesFlagValidation in cli_test.go for flag validation.
    • Add TestIsIncluded and TestIsExcluded in cli_test.go for helper functions.

This description was created by Ellipsis for 6fb5239. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to 6fb5239 in 59 seconds. Click for details.
  • Reviewed 247 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. cmd/tscli/delete/devices/cli.go:82
  • Draft comment:
    Consider abstracting the mutually exclusive flag validation into a helper for improved clarity if additional flags are added.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. cmd/tscli/delete/devices/cli.go:134
  • Draft comment:
    Add a clarifying comment on filter precedence; note that the inclusion filter is applied before exclusion and ephemeral filtering.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
3. cmd/tscli/delete/devices/cli.go:233
  • Draft comment:
    The isIncluded function is nearly identical to isExcluded. Consider refactoring common filter logic if matching rules become more complex.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
4. cmd/tscli/delete/devices/cli_test.go:75
  • Draft comment:
    Good use of dependency injection by overriding newClient in tests to isolate client creation.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
5. cmd/tscli/delete/devices/cli_test.go:60
  • Draft comment:
    Consider adding tests that simulate deletion errors (e.g., deletion failures) to further strengthen test coverage.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_Fs0BA5m5GXkVBEov

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

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.

1 participant