-
Notifications
You must be signed in to change notification settings - Fork 80
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
Int as_i32 feature for backward compliant GraphQL spec compliance #65
base: master
Are you sure you want to change the base?
Conversation
…eanly to GraphQL spec here: https://graphql.org/learn/schema/#scalar-types
impl Number { | ||
#[cfg(not(feature = "as_i32"))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feature flags should be additive. We can still provide as_i64
even with an i32
because that has an impl Into
.
I'll go through and change it around a bit. I leaned too heavily on minimal
change over correctness.
…On Thu, Apr 28, 2022, 7:43 PM Ben Boeckel ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/common.rs
<#65 (comment)>
:
> impl Number {
+ #[cfg(not(feature = "as_i32"))]
Feature flags should be additive. We can still provide as_i64 even with
an i32 because that has an impl Into.
—
Reply to this email directly, view it on GitHub
<#65 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AQTIR7TEHJNXGMZTK6UDX6TVHMPA3ANCNFSM5UTSF2HQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
FWIW, a CI job that runs with the feature flag would also likely be appreciated. |
@ittorchbearer or @mathstuf, any update on this? |
Waiting on updates from the author AFAIK. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Waiting on author changes
(@ittorchbearer FYI) |
added as_i32 backward-compatible feature flag to allow running to GraphQL spec here: https://graphql.org/learn/schema/#scalar-types