Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Improve flags handling [PC-13534] #177

Merged
merged 7 commits into from
Aug 2, 2024

Conversation

nieomylnieja
Copy link
Contributor

@nieomylnieja nieomylnieja commented Aug 1, 2024

Motivation

For a long time sloctl had -p and -A flags available in the global scope, even if specific sub-commands were not using it.
Similarly, -l flag for get <object> command was available even for objects which do not support labels filtering.
This PR finally fixes these issues and sets the aforementioned flags contextually, only when it makes sense.

Testing

  • Ensure all the -p flag usage works for project scoped objects on delete (by name) and get commands.
  • Ensure -A flag is only visible/usable for get command for project scoped objects, and that it works.
  • Ensure -l flag for labels filtering is only visible for objects supporting labels, and that it works.

Successful run: https://github.com/nobl9/sloctl/actions/runs/10200396849/job/28219757765

Breaking Changes

get command exposes -l flag for objects which support labels filtering, in addition -p and -A flags are now only available for Project scoped objects.
delete <name> command no longer supports -A flag and supports -p flag only for Project scoped objects.
get alert command now prints No resources found. notice, just like the rest of get <object> commands.

@nieomylnieja nieomylnieja changed the title feat: Improve flags handling feat: Improve flags handling [PC-13534] Aug 1, 2024
Copy link
Member

@skrolikiewicz skrolikiewicz left a comment

Choose a reason for hiding this comment

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

👍

@daniel-nobl9 daniel-nobl9 added the passed-testing Added by QA when tests have passed label Aug 2, 2024
@nieomylnieja nieomylnieja merged commit 55eece4 into main Aug 2, 2024
8 checks passed
@nieomylnieja nieomylnieja deleted the PC-13534-disallow-A-flag-for-delete-by-name branch August 2, 2024 09:02
nieomylnieja added a commit that referenced this pull request Aug 12, 2024
## Motivation

Alerts, while not directly Project scoped, support Project filtering.
With the changes introduced in #177,
the flag was removed.
This PR brings it back.

## Testing

Build sloctl with `make build` locally and run `sloctl get alert -A` and
`sloctl get alert -p <project_name>`.

## Release Notes

`sloctl get alert` once again supports both `-p` and `-A` flags for
Project filtering.
These flags were removed as a regression in v0.4.0.
nieomylnieja added a commit that referenced this pull request Aug 20, 2024
#177 removed default project flags,
unfortunately we missed the `sloctl replay` command which also relied on
them.

This PR brings the `-p` flag back for `sloctl replay` command.

---------

Co-authored-by: Tomek Labuk <89924840+labtom@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change enhancement New feature or request go minor passed-testing Added by QA when tests have passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants