Skip to content

Conversation

renebrandel
Copy link
Contributor

@renebrandel renebrandel commented Nov 4, 2022

Description of changes:

  • Updated all predicates to use new format
  • Changed code relational models sections
  • Expanded the many-to-many section

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@renebrandel renebrandel requested review from a team as code owners November 4, 2022 23:40
Copy link
Contributor

@abdallahshaban557 abdallahshaban557 left a comment

Choose a reason for hiding this comment

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

Left 1 piece of feedback

Copy link
Member

@svidgen svidgen left a comment

Choose a reason for hiding this comment

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

Need to address statement around "implicit and" groups, which are no longer a thing.

Also missing predicate updates for sync expressions on Syncing data to cloud page. Missing predicate updates for Angular and Vue snippets.

@@ -1,7 +1,8 @@
When using multiple conditions, there is an implicit `and` defined to mirror the [GraphQL Transformer condition statements](/cli/graphql/data-modeling). For example with multiple conditions:
Copy link
Member

Choose a reason for hiding this comment

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

We no longer have the implicit and. All logical groupings must be explicit, and probably worth calling that change out explicitly.

Copy link
Member

Choose a reason for hiding this comment

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

Need to address statement around "implicit and" groups, which are no longer a thing.

Also missing predicate updates for sync expressions on Syncing data to cloud page. Missing predicate updates for Angular and Vue snippets.

I've pushed updates here for selective sync + Angular and Vue snippets.

Leaving the mention of "implicit and" conditions for you — I'm not sure what you want in place of that.

svidgen
svidgen previously approved these changes Nov 9, 2022
stocaaro
stocaaro previously approved these changes Nov 9, 2022
Copy link
Contributor

@abdallahshaban557 abdallahshaban557 left a comment

Choose a reason for hiding this comment

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

LGTM!

@stocaaro stocaaro merged commit 2925d6e into main Nov 14, 2022
@stocaaro stocaaro deleted the renbran/lazy-loading branch November 14, 2022 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants