- 
                Notifications
    You must be signed in to change notification settings 
- Fork 35
Added superfluous optional notification #175
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
Conversation
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, @JoelBergstrand. It looks good. I added a small suggestion and a comment.
| ===== | ||
|  | ||
| [#_neo_clientnotification_statement_superfluousoptional] | ||
| === Superfluous Optional | 
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.
| === Superfluous Optional | |
| === Superfluous optional | 
| |Neo4j code | ||
| m|Neo.ClientNotification.Statement.SuperfluousOptional | ||
| |Title | ||
| a|The use of OPTIONAL is superfluous when `CALL` is a unit subquery or a void procedure. | 
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.
| a|The use of OPTIONAL is superfluous when `CALL` is a unit subquery or a void procedure. | |
| a|The use of `OPTIONAL` is superfluous when `CALL` is a unit subquery or a void procedure. | 
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 should also be changed in the code if we change it here.
| |Title | ||
| a|The use of OPTIONAL is superfluous when `CALL` is a unit subquery or a void procedure. | ||
| |Description | ||
| |The use of OPTIONAL is superfluous when `CALL` is a unit subquery or a void procedure. | 
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.
| |The use of OPTIONAL is superfluous when `CALL` is a unit subquery or a void procedure. | |
| |The use of `OPTIONAL` is superfluous when `CALL` is a unit subquery or a void procedure. | 
| |GQLSTATUS code | ||
| m|03N61 | ||
| |Status description | ||
| a|info: superfluous optional. The use of optional is superfluous when `CALL` is a unit subquery or a void procedure. | 
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.
| a|info: superfluous optional. The use of optional is superfluous when `CALL` is a unit subquery or a void procedure. | |
| a|info: superfluous optional. The use of `OPTIONAL` is superfluous when `CALL` is a unit subquery or a void procedure. | 
| Thanks @renetapopova for the comments. We decided to split the notification in two, I've added examples two both of them and capitalized. Would appreciate you comments on the wording of the notifications and descriptions. | 
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.
Hey @JoelBergstrand, I added some editorial suggestions, mainly regarding consistency with the rest of the page and the monorepo. The docs should reflect the code because we'll be adding tests at some point to make sure all notifications and their details are up-to-date. So, any discrepancies between the docs and the neo4j repository will lead to failures.
| .Redundant use of optional on a procedure call. | ||
| [.tabbed-example] | ||
| ===== | ||
| [.include-with-neo4j-code] | 
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.
You must also add a tab example for the GQLSTATUS code, similar to the rest of the notifications.
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 I've misunderstood the format. I'm pushing some changes to align with the rest of the notifications. I'll make sure that the notifications in the monorepo correspond to what is written here, in substance and formatting.
| |=== | ||
|  | ||
| .Redundant use of optional on a call subquery. | ||
| [.tabbed-example] | 
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 one also needs a tab example for the GQLSTATUS code, similar to the rest of the notifications.
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 I've misunderstood the format. I'm pushing some changes to align with the rest of the notifications. I'll make sure that the notifications in the monorepo correspond to what is written here, in substance and formatting.
Co-authored-by: Reneta Popova <[email protected]>
| Thanks for the documentation updates. The preview documentation has now been torn down - reopening this PR will republish it. | 
| Good morning @renetapopova! Do you have any additional comments on this addition to the docs? | 
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, @JoelBergstrand!
Notification from [OPTIONAL CALL PR](neo-technology/neo4j#26755). --------- Co-authored-by: Reneta Popova <[email protected]>
Notification from OPTIONAL CALL PR.