10
\$\begingroup\$

This question was posted recently, and presents a GraphQL schema for review.

Arguably, GraphQL doesn't do much, and we've closed plain Java and C# posts before where all reviewers had to chew on was a model class with a bunch of properties.

On the other hand, a GraphQL schema is self-contained and formalizes a query interface using JSON objects - and that isn't very different from, say, a T-SQL script that creates tables and their primary and foreign key constraints... and we have reviewed such content before.

GraphQL obviously adheres to JSON standards, and I suppose there are best practices in the design of a GraphQL schema.

So, is GraphQL reviewable?

\$\endgroup\$
3
  • \$\begingroup\$ Do we review JSON models? (Real question, I don't think I've ever stumbled upon it) \$\endgroup\$
    – IEatBagels
    Nov 26, 2019 at 1:53
  • \$\begingroup\$ @IEatBagels good question. I think I'd treat it the same as any other declarative script with a purpose, regardless of language or format. I mean if we review T-SQL CREATE scripts, what's wrong with what looks like (?) the JSON equivalent? \$\endgroup\$
    – Mathieu Guindon
    Nov 26, 2019 at 2:07
  • \$\begingroup\$ Well the SQL script can include indexes and stuff like that, there's somehow a logic that's in my opinion deeper than a template \$\endgroup\$
    – IEatBagels
    Nov 26, 2019 at 13:43

2 Answers 2

Reset to default
8
\$\begingroup\$

GraphQL is on-topic, but only if there is more than just the schema

This is controversial. In my opinion, GraphQL, in general, is reviewable, but the schema on its own, with no implementation detail, is not.

A GhraphQL Schema is analogous to an interface. It is the agreed contract between a client and a server. In GhraphQL, the "implementation" is the resolvers that collect and then supply the data in the schema-conformant presentation.

The controversy is summarized as...

Interfaces on their own are reviewable (some answers say no, but high-voted answers say yes):

Interfaces on their own are not reviewable (but are "design reviews")

And, back to my opinion.

No, interfaces are a specification, not code, and are not suitable for Code Review.

If there was a Design Review stack exchange, then it may be on topic there. In a pinch, if a person wants a style review only of a specification, then perhaps it could be reviewed, but in design phase, it is all too hypothetical to be on-topic

\$\endgroup\$
5
  • 3
    \$\begingroup\$ This is controversial - yep, that's why we're here ;-) \$\endgroup\$
    – Mathieu Guindon
    Nov 26, 2019 at 2:32
  • \$\begingroup\$ "This is controversial. In my opinion, GraphQL, in general, is reviewable, but the schema on its own, with no implementation detail, is not." So, treat it like any DB language? \$\endgroup\$
    – Mast Mod
    Nov 26, 2019 at 6:51
  • 2
    \$\begingroup\$ @Mast - no, don't treat it like any DB language, but, there's a difference between the CREATE TABLE .... statement in an SQL DB, and the SELECT .... FROM .... statement. The one defines the data, the other manipulates the data. For the purposes of Code Review, the columns that the table has, and what relationships are defined between the tables, is more a design review, than a code review. Sure, the design can be reviewed in a Code Review question, but there has to be the actual code that works according to the design first. \$\endgroup\$
    – rolfl
    Nov 26, 2019 at 13:25
  • \$\begingroup\$ So, a GraphQL query is reviewable (ideally with the schema), but the schema itself isn't? \$\endgroup\$
    – Mathieu Guindon
    Nov 26, 2019 at 14:05
  • 1
    \$\begingroup\$ In my opinion, the schema itself is not reviewable if it is on it's own, but with GraphQL, there are 2 other sides, the query that selects from the GraphQL schema, and also the resolvers that supply data behind the schema. If either the supply code, or the consume code is also part of the question, then in my mind, it will be on topic. In database terms, this could be the program that selects data from the table (or the Select), or the data that puts data in to the table (the insert side). The GraphQL Schema is like an on-demand "Table" that you "Select" from that is populated dynamically \$\endgroup\$
    – rolfl
    Nov 26, 2019 at 14:30
2
\$\begingroup\$

Yes, but...

There should be something reviewable. A straight dump of a GraphQL schema is inappropriate, I believe there should be the same rules as SQL.

A straight table-dump of SQL is completely un-reviewable, but a dump of several tables, with relationships and complicated interconnections is reviewable. A dump of a stored procedure, function or trigger is also reviewable. A batch is reviewable.

I believe GraphQL should fit that same general guideline. Questions like "is this GraphQL type sensible" don't fit well, but questions like "does this social network model look reasonable" would be a better fit.

\$\endgroup\$

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .