Skip to content

Comments

Adds ACL categories to each command#1291

Closed
itamarhaber wants to merge 1 commit intomasterfrom
acl-cats
Closed

Adds ACL categories to each command#1291
itamarhaber wants to merge 1 commit intomasterfrom
acl-cats

Conversation

@itamarhaber
Copy link
Member

No description provided.

@oranagra
Copy link
Member

@itamarhaber i took a quick look at the changes, the diff is very hard to review, some commands seem to have moved to a different place in the file, makes it hard to review, but also introduces unnecessary conflicts for this and other PRs.

is it just about specifying the ACL categories for each command? or does this PR include other changes?
about the ACL categories, i'm not certain we wanna document them, same as command flags (ok-loading and such), they might change and we'll need to keep updating the docs.
instead there's a simple command that let's the people get the categories from a the actual server.

@zuiderkwast
Copy link
Contributor

Would we instead want to list the categories automatically by querying the local Redis instance? That would eliminate the need to update the docs when ACL categories change.

@oranagra
Copy link
Member

oranagra commented Aug 1, 2021

I don't think that's needed. I think what we have now is enough. If someone wants the exact details, depending on the version he uses, he should get that from Redis..
Like Salvatore said in the (now deleted) docs: listing them would be boring...

@zuiderkwast zuiderkwast added the to-be-closed should probably be dismissed sooner or later label Aug 1, 2021
@itamarhaber
Copy link
Member Author

Closing as this was implemented elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

to-be-closed should probably be dismissed sooner or later

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants