shurcooL / githubv4

Package githubv4 is a client library for accessing GitHub GraphQL API v4 (https://docs.github.com/en/graphql).
MIT License
1.11k stars 89 forks source link

Add support for Schema Previews #45

Open djoksimo opened 5 years ago

djoksimo commented 5 years ago

Resolving this

@ItsMeWithTheFace and I figured out a way to autogenerate enums and input structs for schema previews by attaching custom media types to the headers.

We have not yet figured out a stable way of automatically fetching the media types (application/vnd.github.audit-log-preview+json, application/vnd.github.antiope-preview+json , etc.) directly from GitHub.

I will add a separate package githubv4preview that attaches these media types and I will add tests for Querying Hovercard Data once I get some feedback on my changes

dmitshur commented 5 years ago

Thanks for looking into this, it's a good start.

I will add a separate package githubv4preview

One of the most difficult decisions we'll need to make before a change like this can be merged is where to place the preview schema types. Should they go into the same package? A new package? Several new packages? We can come back to this later.

I suggest don't worry about making code changes until we agree on the plan.

We have not yet figured out a stable way of automatically fetching the media types (application/vnd.github.audit-log-preview+json, application/vnd.github.antiope-preview+json , etc.) directly from GitHub.

I noticed at https://developer.github.com/v4/public_schema/ GitHub offers the file https://developer.github.com/v4/public_schema/schema.public.graphql. That file has a different format, I'm not completely sure which format it is. But it includes information about all previews, and what identifiers belong to what preview. E.g.:

"""
Marks an element of a GraphQL schema as only available via a preview header
"""
directive @preview(
  """
  The identifier of the API preview that toggles this field.
  """
  toggledBy: String
) on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION

And:

"""
Location information for an actor
"""
type ActorLocation @preview(toggledBy: "audit-log-preview") {
  [...]
}

"""
Represents a 'added_to_project' event on a given issue or pull request.
"""
type AddedToProjectEvent implements Node {
  """
  Identifies the actor who performed the event.
  """
  actor: Actor

  [...]

  """
  Project referenced by event.
  """
  project: Project @preview(toggledBy: "starfox-preview")

  [...]
}

Perhaps we should use that file to generate from, then we can know which types/fields belong to a preview, and add documentation saying that a given Go identifier is a part of a preview.

If that's possible, it may actually make sense to put all the preview types into the main package. But still need to think more about that.

Would you like to investigate (perhaps on a different branch/PR) what it'd take to generate via that file instead?

djoksimo commented 5 years ago

I'll get started on that new PR! Thank you for the feedback and suggestions 😄

djoksimo commented 5 years ago

The https://developer.github.com/v4/public_schema/schema.public.graphql file is in GraphQL SDL format.

I'm still new to Go and GraphQL so creating a transpiler for SDL -> Go lang is definitely a challenge that I'd like to take on, but will take me a long time.

As we require this functionality ASAP, is it cool if I create a parser that retrieves the preview media types (antiope-preview, echo-preview, hawkgirl-preview, etc.) from https://developer.github.com/v4/public_schema/schema.public.graphql instead of hardcoding these values in https://github.com/shurcooL/githubv4/blob/3a0c9c664e28dac70676c9124d28ee57658887c9/gen.go#L25

dmitshur commented 5 years ago

I'm still new to Go and GraphQL so creating a transpiler for SDL -> Go lang is definitely a challenge that I'd like to take on, but will take me a long time.

No worries. You also don't have to work on it unless/until you want to.

As we require this functionality ASAP, is it cool if I create a parser that retrieves the preview media types

It is cool, but I won't be able to merge it into this repository because I'm looking to find a long-term solution here. I suggest you use a fork until this issue is resolved upstream. That way, you can solve your immediate needs without being blocked. This repo doesn't change much, so there shouldn't be much of a maintenance overhead.

djoksimo commented 5 years ago

Hey @dmitshur!

I came across this go-github file

It looks like the rest version of this package uses a similar approach as this PR.

What are your thoughts on the go-github implementation of the preview mode headers?

gleich commented 4 years ago

@djoksimo, @dmitshur any update on this?