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

RFC: All GraphQL API servers have a root `_schema.graphql` file #31

Closed
orta opened this Issue Aug 31, 2018 · 6 comments

Comments

Projects
None yet
3 participants
@orta
Copy link
Member

orta commented Aug 31, 2018

Proposal:

Metaphysics, Exchange, Convection and Gravity (for GravQL) and (any others I can't think of all, impulse/pulse?) have a _schema.graphql file in the root of their respective repos. We add a pre-commit hook that ensures the schema is always up-to-date.

Reasoning

Mainly to move PR review up a level of abstraction to easily discuss schema design, secondary to improve tooling.

  1. Doing this means that people can see directly in the PR, in a language agnostic way, the changes to an API schema
  2. _schema.graphql is a weird name yes, but I want it to be at the top of the PR every time.
  3. Having this file means that we can build consistent tooling around all the repos. Without thinking too deeply:
    • Metaphysics stitching updates can be scripted in MP
    • Client schema updates can be trivially scripted too
    • Peril can poke #graphql when there are schema changes in any repo (easing comms burden on folks)
    • Peril can offer weekly changelogs of all schemas
    • We can build cross-language linter rules like "please always use unions in mutation"

Exceptions:

Nothing I can think of? Probably worth adding a CI check though incase people decide to skip the

Additional Context:

A hat-tip to @cjoudrey who brought this idea up and discussed its merits within GitHub's API at the NYC GraphQL meetup.

@peril-staging peril-staging bot added the RFC label Aug 31, 2018

@cjoudrey

This comment has been minimized.

Copy link

cjoudrey commented Aug 31, 2018

A hat-tip to @cjoudrey who brought this idea up and discussed its merits within GitHub's API at the NYC GraphQL meetup.

Glad you found this tip useful. 😄

_GraphQL.schema is a weird name yes, but I want it to be at the top of the PR every time.

You may want to use the .graphql extension in order to obtain syntax highlighting in GitHub and in editors. Maybe _schema.graphql?

@orta

This comment has been minimized.

Copy link
Member

orta commented Aug 31, 2018

You may want to use the .graphql extension in order to obtain syntax highlighting in GitHub and in editors. Maybe _schema.graphql?

Thanks, updated!

@ashkan18

This comment has been minimized.

Copy link
Contributor

ashkan18 commented Sep 4, 2018

Nice! 👍 do you have an example PR that includes this? I can update Exchange with this.

@orta

This comment has been minimized.

Copy link
Member

orta commented Sep 4, 2018

I don't, was gonna give it a few more days then I'll add it to all repos 👍

@orta

This comment has been minimized.

Copy link
Member

orta commented Sep 7, 2018

Resolution

We decided to do it.

Level of Support

2: Positive feedback.

Additional Context:

I think for Metaphysics we may still need to keep the JSON version around for our ruby apps, (see
github/graphql-client#16 )9 but we can use .gitattributes to ignore those in PRs.

Next Steps

I'll add this to metaphysics this week, and add it to all the other projects using GraphQL next week ideally.

Exceptions

None that I know of.

@orta orta closed this Sep 7, 2018

@orta

This comment has been minimized.

Copy link
Member

orta commented Sep 7, 2018

This is shipped in Metaphysics with artsy/metaphysics@dd8b014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment