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.

Sign up for GitHub

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

Jump to bottom

Make model items accessible via serenity::model::ITEM instead of serenity::model::MODULE::ITEM #2419

Closed
wants to merge 2 commits into from
Closed

Make model items accessible via serenity::model::ITEM instead of serenity::model::MODULE::ITEM #2419

wants to merge 2 commits into from

Conversation

kangalio
Copy link
Collaborator

@kangalio kangalio commented May 6, 2023

Fixes #2394

This PR is backwards-compatible, except for renaming model::Error to ModelError

This PR consists of two commits:

  1. moves the re-exports from model::prelude to model itself
  2. changes all paths within serenity to the new style

The first commit is more important. The second commit serves to clean up long paths within serenity and express that using the new short path is recommended for new code.

The second commit is very huge, though, and manually reviewing all its changes is infeasible. If wanted, I can re-do that commit and record my screen. The review work would be reduced to checking the regexes I used. I would also be okay with cutting out the second commit entirely, if that is preferred. If we do keep the second commit, we should add it to .git-blame-ignore-revs

@kangalio
pub use all items in model
339488d 
Supersedes the prelude submodule

No breaking change, except renaming Error to ModelError
@kangalio
Change model::MODULE::X imports to model::X
9106eaf
@github-actions github-actions bot added builder Related to the `builder` module. cache Related to the `cache`-feature. client Related to the `client` module. command_attr Related to the `command_attr` crate. examples Related to Serenity's examples. framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate gateway Related to the `gateway` module. http Related to the `http` module. model Related to the `model` module. utils Related to the `utils` module. labels May 6, 2023
@mkrasnitski
Copy link
Collaborator

mkrasnitski commented May 7, 2023 •
edited

Per #2394 (comment), I'd say this should go in after the release of 0.12. That would give us the opportunity to either deprecate the old paths in some way, or just remove them (by making all the submodules of model private).

I think one way you could test that the second commit caught every use is by doing the above and seeing if there are errors.

@kangalio
Copy link
Collaborator Author

kangalio commented May 7, 2023

Why do we necessarily need to deprecate or remove the old paths?

@kangalio kangalio linked an issue May 9, 2023 that may be closed by this pull request
Flatten serenity::model #2394
Open
@mkrasnitski
Copy link
Collaborator

Why do we necessarily need to deprecate or remove the old paths?

Because imo having types be importable both from model::* and from model::submodule::* doesn't sit right with me. It's best to have one canonical place to import types from. Deprecating at minimum is probably enough to signal to users that they should instead be using the new path.

However, while working on #2429, I realized that I don't think that model should be entirely flattened to be 1-deep, with every possible model type accessible at that level. Instead, it would be better to flatten it to 2 levels deep, such that all types are accessible at the model::submodule::* level. Then, the model::prelude could fill mostly the same purpose as a 1-level flattening.

From what I can tell, the only submodules that are not already flat are model::colour and model::guild.

@kangalio kangalio closed this May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
builder Related to the `builder` module. cache Related to the `cache`-feature. client Related to the `client` module. command_attr Related to the `command_attr` crate. examples Related to Serenity's examples. framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate gateway Related to the `gateway` module. http Related to the `http` module. model Related to the `model` module. utils Related to the `utils` module.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Flatten serenity::model
2 participants
@kangalio @mkrasnitski