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

DOC Update documentation for GraphQL v4 #10325

Merged

Conversation

maxime-rainville
Copy link
Contributor

@maxime-rainville maxime-rainville commented May 19, 2022

This updates some of the existing GraphQL documentation to match up with the new functionality, be a bit clearer, and some formatting/grammar updates.

The new section in the 4.11.0 changelog is mostly copied from the beta/RC changelog but includes a new "What do I need to know to get started?" section linking to relevant additional information (namely how to upgrade custom schemas, and how to build schemas).

Related PR

Parent issue

@GuySartorelli
Copy link
Member

I think you may have committed the wrong file ;p

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

I mentioned this on slack as well but in case you haven't seen that:
I think it would make sense to split this up.
Everything that talks about write permissions and which options are available for building the schema should probably go in the general graphql docs section somewhere (if it's not there already) - we can then just link to that. It will be relevant for anyone picking up silverstripe from scratch who wants to know how they should be building their graphql schema into the forseeable future, not just people upgrading from 3.

I think this doc should be explicitly about the fact that there is now a stable v4, what the implications for that are, and some quick info (and links to more detailed info) about upgrading from v3.

Also it'll be worth looking at https://docs.silverstripe.org/en/4/developer_guides/graphql/upgrading/ which I found when I was thinking about this - whether that should just be absorbed into these docs and that file removed, or just linked to from this doc, etc.... but we should avoid duplication as much as we can.

docs/en/03_Upgrading/04_Upgrading_graphql4.md Outdated Show resolved Hide resolved
@GuySartorelli GuySartorelli force-pushed the pulls/4.10/graphql-upgrade-doc branch from 855da13 to 4d6910f Compare June 1, 2022 06:09
@GuySartorelli GuySartorelli marked this pull request as draft June 1, 2022 06:09
@GuySartorelli GuySartorelli force-pushed the pulls/4.10/graphql-upgrade-doc branch from 4d6910f to 9090339 Compare June 1, 2022 06:13
@GuySartorelli GuySartorelli changed the title DOC Add upgrade guidance for GraphQL v4 DOC Update documentation for GraphQL v4 Jun 1, 2022
@GuySartorelli GuySartorelli force-pushed the pulls/4.10/graphql-upgrade-doc branch 3 times, most recently from eb5aff9 to 3c7381a Compare June 2, 2022 01:39
@GuySartorelli
Copy link
Member

There is an inconsistency in performance in the docs that I'm not sure how to handle, having done no benchmarking myself.

In the original documentation written by UncleCheese (in 03_building_the_schema.md):

A large schema with 50 DataObject classes exposing all their operations can take up to 20 seconds to generate.

In the new docs written by @maxime-rainville which I've added to 05_deploying_the_schema.md:

While benchmarking schema generation performance, we measured that a schema exposing 180 DataObjects with 1600 relations could be built on-demand in less than 6 seconds on a small AWS instance.

This is a massive discrepancy... which is accurate? Are they both somehow accurate? Is it okay to leave the docs saying what seem to be two conflicting statements, and if not, how should this be handled?

@GuySartorelli GuySartorelli force-pushed the pulls/4.10/graphql-upgrade-doc branch 2 times, most recently from ad91379 to 7f3b9f4 Compare June 8, 2022 02:40
maxime-rainville and others added 3 commits June 8, 2022 14:40
The existing upgrading docs are for upgrading to v4, whereas the new docs are more about how to handle the new .graphql-generated directory.
@GuySartorelli GuySartorelli force-pushed the pulls/4.10/graphql-upgrade-doc branch from 7f3b9f4 to 1d02b91 Compare June 8, 2022 02:43
@GuySartorelli GuySartorelli marked this pull request as ready for review June 8, 2022 02:43
@GuySartorelli GuySartorelli force-pushed the pulls/4.10/graphql-upgrade-doc branch from 1d02b91 to 22a08b9 Compare June 8, 2022 02:52
@GuySartorelli GuySartorelli dismissed their stale review June 8, 2022 02:53

Lots has changed

@GuySartorelli GuySartorelli force-pushed the pulls/4.10/graphql-upgrade-doc branch from 22a08b9 to 85fe016 Compare June 8, 2022 04:43
Copy link
Member

@emteknetnz emteknetnz left a comment

Choose a reason for hiding this comment

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

Thanks for quick turn around

Only thing I'm concerned with here is the ordering of the schema generation options.

@GuySartorelli GuySartorelli force-pushed the pulls/4.10/graphql-upgrade-doc branch from 85fe016 to ac1bb24 Compare June 8, 2022 05:20
@emteknetnz emteknetnz merged commit 22d992a into silverstripe:4 Jun 8, 2022
@emteknetnz emteknetnz deleted the pulls/4.10/graphql-upgrade-doc branch June 8, 2022 05:24
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.

None yet

3 participants