Skip to content

GraphQL: Add schema printer options#23180

Merged
2 commits merged intoDefinitelyTyped:masterfrom
mattleff:patch-1
Jan 25, 2018
Merged

GraphQL: Add schema printer options#23180
2 commits merged intoDefinitelyTyped:masterfrom
mattleff:patch-1

Conversation

@mattleff
Copy link
Copy Markdown
Contributor

@mattleff mattleff commented Jan 25, 2018 •
edited
Loading

These were added with graphql/graphql-js#927, RFC: Descriptions as strings.

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).
  • Provide a URL to documentation or source code which provides context for the suggested changes: RFC: Descriptions as strings. graphql/graphql-js#927
  • [?] Increase the version number in the header if appropriate.

@mattleff
Add schema printer options
768ba04 
These were added with graphql/graphql-js#927, RFC: Descriptions as strings.
@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Jan 25, 2018 •
edited
Loading

@mattleff Thank you for submitting this PR!

🔔 @TonyPythoneer @calebmer @intellix @firede @kepennar @freiksenet @IvanGoncharov @DxCx @rportugal - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

@typescript-bot typescript-bot added Popular package This PR affects a popular package (as counted by NPM download counts). Awaiting reviewer feedback labels Jan 25, 2018
intellix
intellix reviewed Jan 25, 2018
View reviewed changes
Comment thread types/graphql/utilities/schemaPrinter.d.ts Outdated

export function printSchema(schema: GraphQLSchema): string;
export interface PrinterOptions {
[key: string]: any;
Copy link
Copy Markdown
Contributor

@intellix intellix Jan 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct? was looking at this line https://github.com/graphql/graphql-js/pull/927/files#diff-71ba52e9c625f826d2b0df2963c8633aR28

Copy link
Copy Markdown
Contributor Author

@mattleff mattleff Jan 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@intellix I was thinking to leave the options open-ended so that when they add another option we don't have to update types again, but I'm fine making them explicit: 2bb2a2f.

@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Jan 25, 2018

A definition author has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

@ghost ghost merged commit 65e61a6 into DefinitelyTyped:master Jan 25, 2018
@mattleff mattleff deleted the patch-1 branch January 25, 2018 18:09
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@intellix intellix intellix approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Popular package This PR affects a popular package (as counted by NPM download counts).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mattleff @typescript-bot @intellix