-
Notifications
You must be signed in to change notification settings - Fork 83
Show columns clarifications #1826
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
Show columns clarifications #1826
Conversation
to be more visible as tables and not just part of the text, this is what most show commands does also make the examples proper examples
to be more visible as tables and not just part of the text, this is what most show commands does
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @Hunterness. We have started reviewing and restyling all pages moved over from the Cypher Manual to make them coherent with the rest of the pages. So, as part of this effort, this page will also be reviewed. In the meantime, see my comments on your PR.
modules/ROOT/pages/authentication-authorization/manage-privileges.adoc
Outdated
Show resolved
Hide resolved
modules/ROOT/pages/authentication-authorization/manage-privileges.adoc
Outdated
Show resolved
Hide resolved
modules/ROOT/pages/authentication-authorization/manage-privileges.adoc
Outdated
Show resolved
Hide resolved
|
|
||
| When omitting the `AS COMMANDS` clause, results will include multiple columns describing privileges: | ||
| When omitting the `AS COMMANDS` clause, results will include multiple columns describing privileges instead. | ||
| They are all returned by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds like some of the details are not returned by default. I would suggest to remove it.
| They are all returned by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean? they are all default columns returned without a YIELD which is what this is marking. A lot of other tables like this has either another column (or two) showing if they are returned by default for the command or need a yield or they have the default label on them. But since that would be true for all rows in this table we just have this comment instead.
If this is about the column only returned by the SHOW USER PRIVILEGE versions, then it is still a default column by the definition we use it for (not requiring YIELD) it is just not available for all versions of the command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't get from the message that the default was without YIELD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it comes from that being the language we have used throughout the admin command sections, we talk about what is returned by default and what columns you need a YIELD for 🤷
modules/ROOT/pages/authentication-authorization/manage-roles.adoc
Outdated
Show resolved
Hide resolved
| | User name | ||
| | STRING | ||
| |=== | ||
| Since this gives a result with one row for each user, if a role is assigned to two users it will show up twice. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Since this gives a result with one row for each user, if a role is assigned to two users it will show up twice. | |
| Since this gives a result with one row for each user, it shows up twice if a role is assigned to two users. |
|
Thanks for the documentation updates. The preview documentation has now been torn down - reopening this PR will republish it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks
Update the column information to not just be in the text but as proper tables for the 2 show commands that didn't have them as tables already.
Update the column information to not just be in the text but as proper tables for the 2 show commands that didn't have them as tables already. Cherry-picked from #1826 Co-authored-by: Therese Magnusson <[email protected]>
Update the column information to not just be in the text but as proper tables for the 2 show commands that didn't have them as tables already.