Skip to content
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

Union ordering no longer deprecated #219

Closed
wants to merge 1 commit into from

Conversation

gem-neo4j
Copy link
Contributor

@gem-neo4j gem-neo4j commented Nov 18, 2024

We decided to undeprecate this :)

related to https://github.com/neo-technology/neo4j/pull/28207

@gem-neo4j gem-neo4j added the dev The default branch. label Nov 18, 2024
@neo-technology-commit-status-publisher
Copy link
Collaborator

Thanks for the documentation updates.

The preview documentation has now been torn down - reopening this PR will republish it.

@NataliaIvakina
Copy link
Collaborator

NataliaIvakina commented Nov 19, 2024

@gem-neo4j, is this undeprecation for the 5.26 LTS release or 2025.01?
Do I understand correctly that the notification won't be thrown starting from a specific version (5.26 or 2025.01)?

@gem-neo4j
Copy link
Contributor Author

@NataliaIvakina For the 5.26LTS release

Copy link
Collaborator

@NataliaIvakina NataliaIvakina left a comment

Choose a reason for hiding this comment

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

I think everything looks good. If I understood everything correctly, the only thing to do here is to remove the example.
Also, I suppose you have to update the Cypher manual. This deprecation is mentioned here.

@gem-neo4j
Copy link
Contributor Author

The warning message will show up in the Cypher 5.x versions before 5.26, do you think we should just remove it from there completely or add an update in 5.26 saying the deprecation has been removed?

@NataliaIvakina
Copy link
Collaborator

The warning message will show up in the Cypher 5.x versions before 5.26, do you think we should just remove it from there completely or add an update in 5.26 saying the deprecation has been removed?

I think you should add an update in 5.26. Don't remove the deprecation from the list because it is true for those who use Neo4j 5.5-5.25.

Probably in the Status Codes, we also have to add a note or a line to the page https://neo4j.com/docs/status-codes/current/changelogs/. @renetapopova, what do you think?

@gem-neo4j
Copy link
Contributor Author

Like this? neo4j/docs-cypher#1113

@NataliaIvakina
Copy link
Collaborator

Like this? neo4j/docs-cypher#1113

Yes, this looks good (from my side 😄). I'm sure Jens will help you with reviewing the PR for the Cypher manual

@renetapopova
Copy link
Collaborator

The warning message will show up in the Cypher 5.x versions before 5.26, do you think we should just remove it from there completely or add an update in 5.26 saying the deprecation has been removed?

I think you should add an update in 5.26. Don't remove the deprecation from the list because it is true for those who use Neo4j 5.5-5.25.

Probably in the Status Codes, we also have to add a note or a line to the page https://neo4j.com/docs/status-codes/current/changelogs/. @renetapopova, what do you think?

I don't think we can just remove the example and the deprecation. This is a valid deprecation until 5.26. Also, @Lojjs, isn't the removal of this deprecation a breaking change? And yes, we should also update Changes to status codes per Neo4j version to say that this deprecation has been removed in 5.26 or something in this line of thought.

@@ -1440,68 +1438,6 @@ CREATE DATABASE `foo.bar`
======
=====

.Using differently ordered return items in a `UNION` clause
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would rather leave the example and add a note that from 5.26, using differently ordered return items in a UNION [ALL] clause is no longer deprecated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

And the same for the above.

@Lojjs
Copy link
Contributor

Lojjs commented Nov 20, 2024

Also, @Lojjs, isn't the removal of this deprecation a breaking change?

No idea 🤷 It is PM (Valerio I believe) that has decided that this deprecation should be removed. I would maybe let PM decide how to handle the documentation-wise as well. I do not have any strong opinon about this

@NataliaIvakina
Copy link
Collaborator

But do we provide a complete list of all examples for deprecated features here? Union reordering just falls under the notification code Neo.ClientNotification.Statement.FeatureDeprecationWarning, and starting from 5.26 you won't get this warning any longer. The Cypher manual documents this, not the Status Codes. There are no changes to the Neo4j notification code or GQLstatus code.

@gem-neo4j
Copy link
Contributor Author

Removing a deprecation is not considered breaking afaik. Also like Natalia mentioned, that page does not provide a complete list. Including this deprecation there is confusing as it is no longer true and adding a disclaimer feels overkill to mention. I think a user not seeing any mention of this here is the best option :)

@renetapopova
Copy link
Collaborator

Adding Valerio's suggestion here for completeness:
"Add a special subsection called 'Withdrawn deprecations' and put those examples there?
We could write a short intro along the lines:
This section lists features deprecated during this release train that have been un-deprecated after cost/benefit analysis and user conversations. These features will not be withdrawn and will continue to be supported."

@renetapopova
Copy link
Collaborator

Replaced by #224

NataliaIvakina added a commit that referenced this pull request Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.26 dev The default branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants