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

Restrict Snowflake range, type changes, consistent naming #384

Merged
merged 14 commits into from
Sep 5, 2021

Conversation

lukellmann
Copy link
Member

This PR combines #380, #382 and some additional changes. I would want to keep this PR open until discord/discord-api-docs#3745 gets accepted.

Change some types from String to Snowflake (#380)

Restrict Snowflakes to valid range (#382)

Implement the Snowflake limitations described in discord/discord-api-docs#3745. This will e.g. fix an error in pagination (invalid Snowflakes were used before).

The valid range for a Snowflake's raw value is represented by the new validValues property of Snowflake's companion object. All constructors will coerce their input in that range so there is no way to create an invalid Snowflake.
If Discord ever decides to widen the valid range, it would just be a matter of adjusting validValues accordingly to support that. (I intended this range to be the single point of truth).

b96f3bb removes a test that was depending on an invalid Snowflake to pass. Since there is no longer a way to create such invalid Snowflakes, it is impossible to make this test pass.

Consistent naming

  • 957fcc4 renames Snowflake.discordEpochStart to Snowflake.discordEpoch as it is the term used in Discord's documentation
  • fd4f387 brings consistent naming to all XxxTimestamp classes and properties

@HopeBaron HopeBaron merged commit 3c31bbd into kordlib:0.8.x Sep 5, 2021
@lukellmann lukellmann deleted the fixes/snowflake branch February 17, 2022 01:18
@lukellmann lukellmann restored the fixes/snowflake branch February 17, 2022 01:18
@lukellmann lukellmann deleted the fixes/snowflake branch February 17, 2022 01:19
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

Successfully merging this pull request may close these issues.

3 participants