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

We only support 64b systems. Should we also start supporting 32b? #14

Open
PeeHaa opened this issue May 24, 2017 · 2 comments
Open

We only support 64b systems. Should we also start supporting 32b? #14

PeeHaa opened this issue May 24, 2017 · 2 comments
Assignees
Milestone

Comments

@PeeHaa
Copy link
Owner

PeeHaa commented May 24, 2017

Twitter id's don't fit in 32b integers. Should we loosen the type declaration from int to string to support 32b systems?

Related to #12

@DaveRandom
Copy link
Collaborator

My view is that we should start explicitly treating them as strings, as they are identifiers and they will never need to be treated as numbers. This would be a simple enough change, would only require JSON_BIGINT_AS_STRING and (string) casts.

There would be no need to add any additional validation that the string is numeric, since we can assume that anything returned by the API is valid, and in any operations where we send an ID to the API we already need to be prepared for errors that result from passing an identifier that doesn't exist.

This solves the 32-bit issue, and skirts any future issues we might hit because of numeric casting weirdness.

@PeeHaa
Copy link
Owner Author

PeeHaa commented May 26, 2017

Makes total sense. Thanks.

Considering this is a breaking change I will fix this in some future release. Probably https://github.com/aenglander/amp-twitter-bot also need updating so once I made the change we need to PR downstream.

@PeeHaa PeeHaa self-assigned this May 27, 2017
@PeeHaa PeeHaa added this to the v1 milestone May 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants