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

Shouldn't Int translate to an i32? #266

Open
Luro02 opened this issue Aug 31, 2019 · 8 comments
Open

Shouldn't Int translate to an i32? #266

Luro02 opened this issue Aug 31, 2019 · 8 comments
Milestone

Comments

@Luro02
Copy link
Contributor

Luro02 commented Aug 31, 2019

In the documentation it says, that an Int is an i32

Int: A signed 32‐bit integer.

but rust tells me, that an i64 is generated?

error[E0308]: mismatched types
  --> src/main.rs:30:22
   |
30 |             id: Some(id),
   |                      ^^ expected i64, found usize
help: you can convert an `usize` to `i64` and panic if the converted value wouldn't fit
   |
30 |             id: Some(id.try_into().unwrap()),
   |                      ^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: Could not compile `anilist_unofficial`.

To learn more, run the command again with --verbose.
@tomhoule
Copy link
Member

Hmm good point, I don't remember the rationale. Maybe it's in the commit history. This should definitely be investigated, thanks for reporting it.

@mathstuf
Copy link
Contributor

mathstuf commented Oct 1, 2019

It seems that type Int = i64 has been there since the beginning (2024a4f), so nothing in the commit history at least.

I also suspect that type ID = String; is wrong and that it should actually be struct ID(String); (deriving at least Eq I guess) since it is just serialized as a String, but is supposed to be distinct. I don't know how difficult that change would be.

@mathstuf
Copy link
Contributor

@tomhoule Candidate for 1.0?

@tomhoule
Copy link
Member

Definitely, thanks for the ping!

@tomhoule tomhoule added this to the 1.0 milestone Nov 10, 2019
@samyak-jain
Copy link

Is this issue open for contributions?

@mathstuf
Copy link
Contributor

Sure, but it will need a release note to indicate the breaking change. Or changes if you also tackle the ID mismatch.

@samyak-jain
Copy link

Looks like the problem is here: https://github.com/graphql-rust/graphql-parser/blob/8a759df14ff6a2d97b55bcbccad0a53ce0bee4a6/src/common.rs#L46. This needs to be fixed upstream.

@mathstuf
Copy link
Contributor

Thanks. I've filed an issue: graphql-rust/graphql-parser#53

jhult added a commit to jhult/mina-block-explorer that referenced this issue Jun 19, 2024
Fields include: limit, blocks_limit, snarks_limit, trans_limit, epoch, current_epoch, slot_in_epoch, selected_epoch, height, block_height, nonce, and slot

Fixes Granola-Team#600

Related:
- graphql-rust/graphql-client#266
- graphql-rust/graphql-parser#53
jhult added a commit to jhult/mina-block-explorer that referenced this issue Jun 26, 2024
Fields include: limit, blocks_limit, snarks_limit, trans_limit, epoch, current_epoch, slot_in_epoch, selected_epoch, height, block_height, nonce, and slot

Fixes Granola-Team#600

Related:
- graphql-rust/graphql-client#266
- graphql-rust/graphql-parser#53

a
jhult added a commit to jhult/mina-block-explorer that referenced this issue Jun 28, 2024
Fields include: limit, blocks_limit, snarks_limit, trans_limit, epoch, current_epoch, slot_in_epoch, selected_epoch, height, block_height, nonce, and slot

Fixes Granola-Team#600

Related:
- graphql-rust/graphql-client#266
- graphql-rust/graphql-parser#53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants