- 
                Notifications
    You must be signed in to change notification settings 
- Fork 35
Fix deprecation examples #269
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
| ====== | ||
| ===== | ||
|  | ||
| .Using id() function | 
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 confusing part here is that we throw 01N02, but actually, there is a replacement elementId() and it should probably throw 01N01 instead.
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.
If 01N02 is the correct code, I need to move it to the section "Deprecated features without a future replacement".
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.
There is a Trello card about this https://trello.com/c/7yJJyG5q/995-deprecated-function-id-needs-a-better-error-message. elementId is not a direct replacement, it works in some situations and in some not. This whole deprecation is a very debated topic 😓 I think it might end up having 01N00 once that is implemented. For now 01N02 is what it has in the 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.
OK, I understand. I'll move it under the Deprecated features without a future replacement section.
By the way, in the Cypher Manual, it said that id() is replaced by elementId(). Here https://neo4j.com/docs/cypher-manual/current/functions/#header-query-functions-scalar. I guess we need to update that as well. This card is from 27 Aug 2023 😄.
16e1517    to
    75e5eb3      
    Compare
  
    …thout a replacement
| 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.
https://github.com/neo-technology/neo4j/pull/29918 has been merged, so you probably want to align with that. :-)
| Returned GQLSTATUS code:: | ||
| 01N01 | ||
|  | ||
| Returned status description:: | ||
| warn: feature deprecated with replacement. | ||
| `id` is deprecated. | ||
| It is replaced by `elementId()`. | ||
|  | ||
| Suggestions for improvement:: | ||
| Use the function `elementId()` instead. | ||
| + | ||
| [source,cypher] | ||
| ---- | ||
| MATCH (a) | ||
| RETURN elementId(a) | ||
| ---- | ||
| ====== | ||
| [.include-with-GQLSTATUS-deprecated-without-replacement] | ||
| ====== | ||
| Query:: | ||
| + | ||
| [source,cypher] | ||
| ---- | ||
| MATCH (a) | ||
| RETURN id(a) | ||
| ---- | ||
|  | ||
| Returned GQLSTATUS code:: | ||
| 01N02 | ||
|  | ||
| Returned status description:: | ||
| warn: feature deprecated without replacement. | ||
| `id` is deprecated and will be removed without a replacement. | 
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.
Given that https://github.com/neo-technology/neo4j/pull/29918 is merged now, you should revert the change regarding the id function and adjust to the message that points not only to elementId but also "an application-generated id".
It is replaced by elementId or an application-generated id
| 
 Yes, thanks, @hvub | 
| Replaced by #286 | 
No description provided.