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

Replace 'find{Breaking,Dangerous}Changes' with generic 'findSchemaChanges' #2181

Closed
wants to merge 2 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/utilities/findBreakingChanges.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,11 @@ export function findDangerousChanges(
return ((dangerousChanges: any): Array<DangerousChange>);
}

function findSchemaChanges(
/**
* Given two schemas, returns an Array containing descriptions of all the types
* of potentially breaking change and dangerous change
*/
export function findSchemaChanges(
Copy link
Member

Choose a reason for hiding this comment

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

@wtrocki Totally reasonable 👍
Moreover, I think it's confusing to have multiple functions that basically do the same so
I think findSchemaChanges should be a function to rule them all.

Since we working on new major release 15.0.0 I think its good time to remove findDangerousChanges and findBreakingChanges. And merge BreakingChange | DangerousChange into single SchemaChange type.

Can you please do that?

Also, don't forget to update TS typings in tstypes folder.
Another thing all public APIs should be exported through src/index.js and src/utilities/index.js.
https://github.com/graphql/graphql-js/blob/master/src/README.md

oldSchema: GraphQLSchema,
newSchema: GraphQLSchema,
): Array<BreakingChange | DangerousChange> {
Expand Down