Skip to content

reviewing.md

Latest commit

 

History

History
96 lines (57 loc) · 4.16 KB

reviewing.md

File metadata and controls

96 lines (57 loc) · 4.16 KB
stage none
group unassigned
info To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments

GraphQL API merge request checklist

The GitLab GraphQL API has a fair degree of complexity so it's important that merge requests containing GraphQL changes be reviewed by someone familiar with GraphQL. You can ping one via the @gitlab-org/graphql-experts group in a MR or in the #f_graphql channel in Slack (available to GitLab team members only).

GraphQL queries need to be reviewed for:

  • breaking changes
  • authorization
  • performance

Review criteria

This is not an exhaustive list.

Description with sample query

Ensure that the description includes a sample query with setup instructions. Try running the query in GraphiQL on your local GDK instance.

No breaking changes (unless after full deprecation cycle)

Check the MR for any breaking changes.

If a feature is marked as an Experiment, you can make breaking changes immediately, with no deprecation period.

For more information, see deprecation and removal process.

Multiversion compatibility

Ensure that multi-version compatibility is guaranteed. This generally means frontend and backend code for the same GraphQL feature can't be shipped in the same release.

For details, see multiple version compatibility.

Technical writing review

Changes to the generated API docs require a technical writer review.

Changelog

Public-facing changes that are not marked as an Experiment require a changelog entry.

Use the framework

GraphQL is a framework with many moving parts. It's important that the framework is followed.

  • Do not manually invoke framework bits. For example, do not instantiate resolvers during execution and instead let the framework do that.
  • You can subclass resolvers, as in MyResolver.single (see deriving resolvers).
  • Use the ready? method for more complex argument logic (see correct use of resolver#ready).
  • Use the prepare method for more complex argument validation (see validating arguments).

For details, see resolver guide.

Authorization

Ensure proper authorization is followed and that authorize :some_ability is tested in the specs.

For details, see authorization guide.

Performance

Ensure:

Use appropriate types

For example:

  • TimeType for Ruby Time and DateTime objects.
  • Global IDs for id fields

For details, see types.

Appropriate complexity

Query complexity is a way of quantifying how expensive a query is likely to be. Query complexity limits are defined as constants in the schema. When a resolver or type is expensive to call we need to ensure that the query complexity reflects that.

For details, see max complexity, field complexity and query limits.

Testing

  • Resolver (unit) specs are deprecated in favour of request (integration) specs.
  • Many aspects of our framework are outside the resolve method and a request spec is the only way to ensure they behave properly.
  • Every GraphQL change MR should ideally have changes to API specs.

For details, see testing guide.