graphql-inspector: Pull Request behind the target branch causing issues

I don’t understand why you want to do it. A Pull Request is defined to be merged into master, so you need to know if the PR won’t break anything. If you compare it to something different than what’s the point?

Could you explain a bit more? Maybe I don’t understand something?

Apologies @kamilkisiela, I managed to miss your reply here.

Suppose you have two developers, Bob & Sue, both working on the same codebase. They both pull the code at the same time to begin their work – let’s call the commit they start with #c0.

After a day, Sue commits a change where she adds a new field to the schema (Person.age for arguments sake), so that the latest commit on master is now #c1.

Bob then pushes a new feature that does not change the schema whatsoever, and so can’t possibly introduce a backwardly incompatible change. However, graphql-inspector compares his commit (which does not yet contain Person.age) against master, which does, and concludes that Bob is breaking backwards compatibility by removing the Person.age field.

Of course, he’s doing no such thing, and the problem is only that the latest version of master (containing commit #c1) was not merged into his branch before performing the comparison.

Hopefully that makes more sense now?

@wopian done, use master and try to run Inspector Action on pull_request event, when it’s on push it won’t get a PR number. I would recommend to use pull_request on PRs and push for master branch or something like that.

@kamilkisiela thank you! Will add this to my TODO list, this and something on our end ended up making me postpone adding it to our actions at the time.

Looks like it’s working perfectly! We’ll keep that flag turned on in our repo and see how that goes! Thanks, again!

Thanks so much! I’ll go give it a shot!

Feedback welcome!

@wopian not yet but I’m working on it now

@tylercrumpton @wopian @euvl @dchambers good news 😃

https://github.com/kamilkisiela/graphql-inspector/pull/1692

This way on every pull request we get the schema from refs/pull/PR_NUMBER/merge which means it’s merged with the target branch. Your scenario should be solved by that change.

sounds reasonable, I will work on a solution then 😃

I’ll second this request! We’ve run into this fairly frequently, with the same circumstances that @dchambers described. We would love to see it compare the merged/rebased commit against the defined schema instead of comparing the tip of the branch against the defined schema.