-
Notifications
You must be signed in to change notification settings - Fork 22
Editorial review #661
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
Editorial review #661
Conversation
|
angrykoala
left a comment
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.
Good job, just a few small changes
|
|
||
| To make a `CREATE` clause, first create a valid pattern using the xref:/patterns.adoc[Pattern] class: | ||
|
|
||
| To add the clause, first create a valid pattern using the xref:/patterns.adoc[Pattern] class: |
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 page relates to CREATE clause, but I feel it may still be confusing to just refer to it as "the clause". In many places we need to either use multiple clauses or we may reference other clauses, so even at the risk of it being a bit verbose, I'd prefer for the create to be explicitly referenced
| `.set` take tuples of 2 elements, the first being the property to update and the second the value: | ||
| `.set` takes a tuple of two elements - a property to update and a new value: |
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 may be hard to explain, but the method take multiple tuples, so the proposed change is not accurate
So, for example:
.set(
[movie.property("title"), new Cypher.Param("The Matrix")], // Tuple 1
[movie.property("description"), new Cypher.Param("A description")] //Tuple 2
)
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.
rephrased
| This page describes how to create a link:https://neo4j.com/docs/cypher-manual/current/clauses/match/[`MATCH`] clause with the `Cypher.Match` class. | ||
|
|
||
| To create a `MATCH` clause, first create a valid pattern using the xref:/patterns.adoc[Pattern] class and pass it to `Match` constructor: | ||
| To add the clause, first create a valid pattern using the xref:/patterns.adoc[Pattern] class and pass it to the `Match` constructor: |
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.
Same as with CREATE I'd rather specify the clause explicitly
| `ON CREATE SET` and `ON MATCH SET` can follow a `MERGE` statement. | ||
| You can achieve this with the methods `onCreateSet` and `onMatchSet` respectively: |
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.
Following the pattern in the other changes, isn't more consistent something a bit more like:
use the methods `onCreateSet` .... to add the clauses `ON CREATE SET`...
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.
rephrased
Co-authored-by: angrykoala <[email protected]>
|
|
Thanks for the documentation updates. The preview documentation has now been torn down - reopening this PR will republish it. |



part 1 - more commits will follow