Skip to content

Conversation

@renetapopova
Copy link
Contributor

@renetapopova renetapopova commented Jan 6, 2022

When merged and cherry-picked forward, the same structure should be applied to 4.4 and 5.0.
The update in 5.0 depends on 1328 and 1340.

@renetapopova
Copy link
Contributor Author

Just a quick note for you, @Hunterness and @martin-neotech, I haven't changed any wording or done any editorial changes. The only thing you should do is to see if the proposed structure/grouping makes sense to you. Thanks a lot!

Copy link
Contributor

@martin-neotech martin-neotech left a comment

Choose a reason for hiding this comment

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

Thanks, nice work. Found 2 minor suggestions.

[[cypher-deprecations-additions-removals-4.3]]
== Version 4.3

=== Deprecated features
Copy link
Contributor

Choose a reason for hiding this comment

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

I know at least I have added the new syntax followed by the deprecations in the list that was here when introducing/updating the syntax that replaces the deprecation

Example:

label:syntax[] label:updated[]
SHOW [PROPERTY] EXIST[ENCE] CONSTRAINTS

followed by

label:syntax[] label:deprecated[]
SHOW EXISTS CONSTRAINTS

but the way you have structured it now it will come in the opposite order, with the deprecation first and then the replacement being introduced after. Not sure it is an issue just a reflection I had

Copy link
Contributor Author

@renetapopova renetapopova Jan 7, 2022

Choose a reason for hiding this comment

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

Hmm, I haven't changed the order. You can find the original on line 238.
It looks like this:

a|
label:syntax[]
label:deprecated[]
[source, cypher, role="noheader"]
----
SHOW EXISTS CONSTRAINTS
----
a|
Replaced by:
[source, cypher, role="noheader"]
----
SHOW [PROPERTY] EXIST[ENCE] CONSTRAINTS
----
Still allows `BRIEF` and `VERBOSE` but not `YIELD` or `WHERE`.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not what I ment, lets see if I can explain my thoughts better

You ordered the categories as:

deprecated
updated
new

while when we (or at least me, but I think more of us cypher people) added things to this list we would add them in the order

new or updated
deprecated

so we first introduced the replacement and then the deprecation. The description for the deprecation would still include the replacement which is what your piece of code is.

another example would be first introducing dropping index by name (DROP INDEX name) and then deprecating dropping index by label/property combination (DROP INDEX ON :Label(prop)).

Is that any clearer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, now I understood you :) OK, so I changed the order because the requested improvement was to add some kind of structure, especially to the areas that are affected, such as removals and deprecations, because those will break code going forward while additions are mostly FYI.

Copy link
Contributor

@martin-neotech martin-neotech left a comment

Choose a reason for hiding this comment

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

Thanks.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants