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

Make discriminator optional and non-zero in preparation for new usernames #2415

Merged
merged 1 commit into from May 6, 2023

Conversation

Xaeroxe
Copy link
Contributor

@Xaeroxe Xaeroxe commented May 4, 2023

This PR requires a major version bump!

Per https://discord.com/blog/usernames the discriminator field is about to become optional. This will proceed in two phases. It's not clear when phase 1 will start. It may have started already.

Phase 0 (Prior behavior): All users have discriminators.

Phase 1: Users with discriminators co-exist among users without discriminators. Users without discriminators have "discriminator": 0 in their JSON during this phase.

Phase 2: Once all users are migrated, discriminators are eliminated entirely and won't be returned in the JSON.

This PR is designed such that it can continue working correctly regardless of which phase we are currently in.

@github-actions github-actions bot added cache Related to the `cache`-feature. model Related to the `model` module. utils Related to the `utils` module. labels May 4, 2023
@tazz4843
Copy link
Contributor

tazz4843 commented May 4, 2023

Breaking changes should target next, not current.

@Xaeroxe Xaeroxe changed the base branch from current to next May 4, 2023 02:56
@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented May 4, 2023

Alright, let me get these conflicts sorted

@Xaeroxe Xaeroxe force-pushed the discriminator-optional branch 3 times, most recently from edce406 to c15b051 Compare May 4, 2023 03:43
@github-actions github-actions bot removed the cache Related to the `cache`-feature. label May 4, 2023
@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented May 4, 2023

This is ready for review now. I was winging it and I didn't do much in the way of consulting people about API design here. I recognize this is not good practice and I apologize.

@Xaeroxe Xaeroxe force-pushed the discriminator-optional branch 2 times, most recently from 0c8e10d to 9b4cfa5 Compare May 4, 2023 23:08
@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented May 4, 2023

Formatting wasn't correct because I didn't invoke it properly before. It's fixed now.

@arqunis arqunis added enhancement An improvement to Serenity. discord feature Related to Discord's functionality. breaking change The public API is changed, resulting in miscompilations or unexpected new behaviour for users labels May 6, 2023
@arqunis arqunis merged commit aac6b1c into serenity-rs:next May 6, 2023
21 checks passed
@arqunis
Copy link
Member

arqunis commented May 6, 2023

Thanks!

@Xaeroxe Xaeroxe deleted the discriminator-optional branch May 6, 2023 15:36
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 7, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 17, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 17, 2023
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request May 18, 2023
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request May 19, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 21, 2023
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request May 30, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 30, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Jun 3, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Jun 26, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Aug 28, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Sep 6, 2023
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Sep 21, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Sep 21, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Sep 21, 2023
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Oct 17, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Oct 23, 2023
mkrasnitski pushed a commit to mkrasnitski/serenity that referenced this pull request Oct 24, 2023
arqunis pushed a commit to arqunis/serenity that referenced this pull request Oct 24, 2023
arqunis pushed a commit to arqunis/serenity that referenced this pull request Oct 24, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Oct 24, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Nov 1, 2023
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Nov 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change The public API is changed, resulting in miscompilations or unexpected new behaviour for users discord feature Related to Discord's functionality. enhancement An improvement to Serenity. model Related to the `model` module. utils Related to the `utils` module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants