Skip to content

Add deprecations introduced in v6 #249

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

Merged
merged 6 commits into from
Mar 19, 2025
Merged

Add deprecations introduced in v6 #249

merged 6 commits into from
Mar 19, 2025

Conversation

angrykoala
Copy link
Member

  • Remove additions section in migration guide
  • Add new deprecations that have been introduced in version 6

Comment on lines 561 to 567
[source, graphql, indent=0]
----
type Movie implements Production @node {
title: String! @unique
actors: [Actor!]! @relationship(type: "ACTED_IN", direction: IN, properties: "ActedIn")
}
----
Copy link
Contributor

Choose a reason for hiding this comment

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

what can a user do to achieve something similar?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we really provide any alternative to this at the moment. The issue with @unique is that it isn't really supported by neo4j itself, so it is very unreliable

Copy link
Contributor

Choose a reason for hiding this comment

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

could/should we communicate the fact that it was unreliable? (unreliable how? no way of ensuring uniqueness?)

Copy link
Member Author

Choose a reason for hiding this comment

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

There were some cases in which the uniqueness could not be enforced, leading to data that doesn't match the schema. I'm not sure how to properly communicate this

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we usually write motivations for deprecations?

Perhaps we can write something like this:

The @unique directive has been removed as it could not always be reliably enforced, potentially leading to data inconsistencies that did not match the schema.


=== Deprecated `*aggregate` fields

Top level and nested `*Aggregate` fields have been deprecated in favor of `aggregate` fields inside connections:
Copy link
Contributor

@rsill-neo4j rsill-neo4j Mar 18, 2025

Choose a reason for hiding this comment

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

Suggested change
Top level and nested `*Aggregate` fields have been deprecated in favor of `aggregate` fields inside connections:
Top level and nested `*Aggregate` fields have been deprecated in favor of `aggregate` fields inside connections:

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this one was correct, for instance moviesAggregate has the casing as Aggregate

Copy link
Contributor

@rsill-neo4j rsill-neo4j Mar 19, 2025

Choose a reason for hiding this comment

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

in the heading (line 630) it was lower case, likewise in the text following the code listing (line 664).
let's make it consistent then :)

Copy link
Contributor

Choose a reason for hiding this comment

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

The mixed casing seems to be important in this. *Aggregate (note the *) for example moviesAggregate, has been removed, and replaced with a aggregate.

Copy link
Contributor

Choose a reason for hiding this comment

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

so upper case for all "*" ?

Co-authored-by: Richard Sill <156673635+rsill-neo4j@users.noreply.github.com>
@angrykoala angrykoala requested a review from rsill-neo4j March 18, 2025 15:53
Co-authored-by: Richard Sill <156673635+rsill-neo4j@users.noreply.github.com>
@angrykoala angrykoala requested a review from rsill-neo4j March 19, 2025 09:51
angrykoala and others added 2 commits March 19, 2025 10:19
Co-authored-by: Richard Sill <156673635+rsill-neo4j@users.noreply.github.com>
@angrykoala angrykoala requested a review from rsill-neo4j March 19, 2025 11:51
Copy link
Contributor

@rsill-neo4j rsill-neo4j left a comment

Choose a reason for hiding this comment

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

last suggestion + approval

Co-authored-by: Richard Sill <156673635+rsill-neo4j@users.noreply.github.com>
@angrykoala angrykoala merged commit b929bc9 into 6.x Mar 19, 2025
4 checks passed
@neo-technology-commit-status-publisher
Copy link
Collaborator

neo-technology-commit-status-publisher commented Mar 19, 2025

Thanks for the documentation updates.

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

@angrykoala angrykoala deleted the update-migration-guide-6 branch March 19, 2025 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants