Field merging validation: clarify pair members are distinct #1136
Conversation
✅ Deploy Preview for graphql-spec-draft ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
andimarek
changed the title
benjie
added
the
✏️ Editorial
|
What was our process for editorial changes again? How long do we wait after getting some approvals? cc @benjie ? |
|
I generally give it a couple weeks and then merge if it has approvals. (This is already in my queue.) But this one is so trivial, we might as well go ahead and merge it. |
|
I was just reading it again and I don’t think it really matters whether or not they’re distinct, since they will have the same response shape if they’re the same… But more efficient this way 🤷 |
|
@benjie yes it is more performant this way, but maybe even a bit more important it makes it more clear why it should run on every SelectionSet instead of per Operation. If the fields don't need to be distinct you could run the whole validation once. So overall this version I believe make more sense. Thanks for merging! |
Hey ... I hope that can be seen as editorial clarification:
In the overlapping field merging validation the term "pair" is used twice, without making it perfectly clear that the two pair members should be distinct.
In theory they don't have to be different, therefore this PR clarifies it.
Andi