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

Enforce required fields for builders #2087

Merged
merged 11 commits into from
Aug 17, 2022

Conversation

mkrasnitski
Copy link
Collaborator

This PR adds enforcement for unconditionally required fields on builders. It does so by removing the implementation of Default on those builders and adding a new method that takes the required fields as argument. The following builders have been changed as such:

AddMember
CreateChannel
CreateScheduledEvent
CreateSticker
CreateThread
CreateStageInstance
CreateWebhook
CreateApplicationCommand
CreateApplicationCommandOption
CreateButton
CreateInputText
CreateSelectMenu
CreateSelectMenuOption
CreateEmbedAuthor
CreateEmbedFooter

All other builders have retained their implementations of Default, but I've added a new method to them as well that simply wraps Self::default. A few things to note are:

  • EditAutoModRule has been left untouched, however it should be noted that creating an automoderation rule has some required fields, whereas modifying one does not. Potentially it might be worth it to add a new CreateAutoModRule builder, although that would introduce a good bit of code duplication.
  • Renamed EditRole::new to EditRole::from_role to be more descriptive, and changed EditRole::new to wrap EditRole::default.
  • This PR does not add enforcement of conditionally required fields (such as for CreateMessage and others where one of content, embeds, or attachments is required). Doing so would (I think) require using a typestate pattern, so should be done in a separate PR, if at all.

Bikeshedding on parameter order for the newly enforced fields in each Builder::new method is appreciated. I've done my best to order them based on some intuitive sense of hierarchy.

@github-actions github-actions bot added builder Related to the `builder` module. examples Related to Serenity's examples. framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate model Related to the `model` module. labels Aug 9, 2022
@arqunis arqunis added enhancement An improvement to Serenity. breaking change The public API is changed, resulting in miscompilations or unexpected new behaviour for users labels Aug 17, 2022
@arqunis arqunis merged commit f671616 into serenity-rs:next Aug 17, 2022
arqunis pushed a commit that referenced this pull request Aug 21, 2022
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
arqunis pushed a commit that referenced this pull request Sep 2, 2022
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
kangalio pushed a commit to kangalio/serenity that referenced this pull request Sep 11, 2022
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Oct 1, 2022
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Nov 7, 2022
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
arqunis pushed a commit to arqunis/serenity that referenced this pull request Nov 13, 2022
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Feb 28, 2023
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 18, 2023
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request May 30, 2023
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Sep 21, 2023
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Oct 17, 2023
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
mkrasnitski added a commit to mkrasnitski/serenity that referenced this pull request Oct 24, 2023
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
arqunis pushed a commit to arqunis/serenity that referenced this pull request Oct 24, 2023
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
arqunis pushed a commit to arqunis/serenity that referenced this pull request Oct 24, 2023
This commit adds enforcement for unconditionally required fields on builders.
It does so by removing the implementation of `Default` on those builders and
adding a `new` method that takes the required fields as argument.
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 builder Related to the `builder` module. enhancement An improvement to Serenity. examples Related to Serenity's examples. framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate model Related to the `model` module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants