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

unify directives presentation #211

Merged
merged 8 commits into from
Nov 8, 2024
Merged

unify directives presentation #211

merged 8 commits into from
Nov 8, 2024

Conversation

rsill-neo4j
Copy link
Contributor

added a number of "tba" in comments. mostly missing definitions, but on some occasions i wasn't sure which parts of the follow-up sections could got to the "usage" subsection.

Copy link
Contributor

@lidiazuin lidiazuin left a comment

Choose a reason for hiding this comment

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

Looking good, just a few suggestions and questions.

modules/ROOT/pages/directives/custom-logic.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/directives/database-mapping.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/directives/database-mapping.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/directives/indexes-and-constraints.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/directives/indexes-and-constraints.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/directives/indexes-and-constraints.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/security/authorization.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/directives/database-mapping.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/directives/database-mapping.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/directives/database-mapping.adoc Outdated Show resolved Hide resolved
Comment on lines 14 to 16
=== Definition

// tba
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
=== Definition
// tba

It's too complicated to put as a definition.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's a specific version for validate:

[source, graphql, indent=0]
----
"""
This is a simpler version of the authentication directive to be used in the validate-document step.
"""
directive @authentication(
  """operations"""
  operations: [String]
  jwt: String
) on OBJECT | FIELD_DEFINITION | SCHEMA
----

Comment on lines 15 to 17
=== Definition

// tba
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
=== Definition
// tba

As with authorization this is too complicated to write a simple definition for, I think.

There exists a "simple" case for validate:

[source, graphql, indent=0]
----
"""
This is a simpler version of the authorization directive to be used in the validate-document step.
"""
directive @authorization(
  """filter"""
  filter: [String]
  """validate"""
  validate: [String]
) on OBJECT | FIELD_DEFINITION
----

modules/ROOT/pages/security/configuration.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/security/configuration.adoc Outdated Show resolved Hide resolved
Comment on lines 13 to 15
=== Definition

// tba
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
=== Definition
// tba

Same as others.

[source, graphql, indent=0]
----
"""
This is a simpler version of the subscriptionsAuthorization directive to be used in the validate-document step.
"""
directive @subscriptionsAuthorization(
  """filter"""
  filter: [String]!
) on OBJECT | FIELD_DEFINITION
----

@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.

@neo-technology-commit-status-publisher
Copy link
Collaborator

💔 All backports failed

Status Branch Result
6.x Backport failed because of merge conflicts

You might need to backport the following PRs to 6.x:
- wording changes, added a link
7.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

backport --pr 211

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

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.

4 participants