-
Notifications
You must be signed in to change notification settings - Fork 625
Split core functionality into a serenity-core
subcrate
#3387
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
base: next
Are you sure you want to change the base?
Conversation
84fc566
to
1ee1318
Compare
Ran some compile-time benchmarks on my machine using
After this PR:
If we compare mean execution times, that gives a 2.6% improvement for |
7c5f769
to
e49f8ef
Compare
This PR is ready for review now |
c2c534b
to
e2e6d00
Compare
e2e6d00
to
b1be8be
Compare
b1be8be
to
872cb73
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, and a good start to splitting apart serenity.
I would be interested in seeing what the difference in cargo build --timings
looks like, specifically with a poise or songbird based bot to see if they can depend only on core and kick off their build earlier.
I haven't taken a deep look over this, but spotted this comment immediately.
271ba0d
to
235b40d
Compare
235b40d
to
0f64b09
Compare
This PR splits serenity into two crates,
serenity
andserenity-core
.builder
,cache
,http
,model
, andutils
modules.serenity
crate depends on the core and re-exports everything in it, plus it also contains thegateway
,collector
,framework
, andinteractions_endpoint
modules. It also re-exposes all of the core's feature flags, though I'm not sure if that's the right approach.Further splitting may be possible in the future but would require significant refactoring to remove circular dependencies between modules. For error propagation, a bunch of variants on
serenity::Error
got moved into the core, and are propagated usingError::Core
which wraps aserenity_core::Error
. Though, I wonder if it's better to keep the variants and convert 1-1 from aserenity_core::Error
so as to keep things more flat.