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

Use specifiedBy instead of specifiedByUrl #3032

Merged
merged 4 commits into from Apr 20, 2021

Conversation

Code-Hex
Copy link
Contributor

@Code-Hex Code-Hex commented Apr 10, 2021

type __Type {
  kind: __TypeKind!
  name: String
  description: String
  ...
  # should be non-null for custom SCALAR only, must be null for the others
  specifiedBy: String
}
  • But I didn't fix IntrospectionOptions fields because keeping backward compatible.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 10, 2021

CLA Signed

The committers are authorized under a signed CLA.

@Code-Hex
Copy link
Contributor Author

@IvanGoncharov Could you review please 🙇

@IvanGoncharov
Copy link
Member

@Code-Hex Thanks for reporting this issue 👍
It's issue with spec during review process for "[RFC] Custom Scalar Specification URLs" we change directive from @specified(by: "") to @specifiedBy(url: "") but forget to update introspeciton field.
Introspection field should be specifiedByURL to be consistent with deprecatedReason representing @deprecated(reason: "") directive.
graphql/graphql-spec#649 is still Stage3 and changes are not released yet so we should fix it in a spec.

If you are interested, can you please prepare PR against spec and I will help to fast-track it?

@Code-Hex
Copy link
Contributor Author

Code-Hex commented Apr 12, 2021

@IvanGoncharov Thanks for reply!
Oh, I'm interested for this fix!
I will send PR to graphql/graphql-spec that I suggest to replace specifiedBy with specifiedByURL following deprecatedReason 👍

Updated: I have sent PR: graphql/graphql-spec#848 🙇

@IvanGoncharov
Copy link
Member

@Code-Hex Thanks 👍

@Code-Hex
Copy link
Contributor Author

Code-Hex commented Apr 14, 2021

I've prepared this PR is ready to merge always after merged graphql/graphql-spec#848

@Code-Hex
Copy link
Contributor Author

@IvanGoncharov PR is merged! Please review this 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: breaking change 💥 implementation requires increase of "major" version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants