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

Add support for Schema Previews #45

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

djoksimo
Copy link

@djoksimo djoksimo commented Jul 4, 2019

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
Copy link
Member

dmitshur commented Jul 4, 2019

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
Copy link
Author

djoksimo commented Jul 4, 2019

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

@djoksimo
Copy link
Author

djoksimo commented Jul 5, 2019

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

var schemaPreviews = []string{

@dmitshur
Copy link
Member

dmitshur commented Jul 5, 2019

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
Copy link
Author

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
Copy link

gleich commented Sep 2, 2020

@djoksimo, @dmitshur any update on this?

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

Successfully merging this pull request may close these issues.

None yet

3 participants