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

🚨 Wrap large enum variants in Box #484

Merged
merged 1 commit into from
Oct 19, 2024
Merged

🚨 Wrap large enum variants in Box #484

merged 1 commit into from
Oct 19, 2024

Conversation

JMicheli
Copy link
Collaborator

The new version of rust comes with a new clippy lint that triggers on enums with large sizes. Enums have a size equal to their largest contained variant.

The CoreError enum came in at 144 bytes as the result of containing prisma_client_rust::QueryError, which is huge. This is easily fixed by boxing the type, which shut clippy up.

There are some other big variants still, but nothing over 100 bytes.

Copy link

codecov bot commented Oct 19, 2024

Codecov Report

Attention: Patch coverage is 0% with 16 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
core/src/job/error.rs 0.00% 10 Missing ⚠️
core/src/error.rs 0.00% 3 Missing ⚠️
core/src/opds/v2_0/error.rs 0.00% 3 Missing ⚠️
Files with missing lines Coverage Δ
core/src/error.rs 0.00% <0.00%> (ø)
core/src/opds/v2_0/error.rs 38.88% <0.00%> (-7.78%) ⬇️
core/src/job/error.rs 0.00% <0.00%> (ø)

Copy link
Collaborator

@aaronleopold aaronleopold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@aaronleopold aaronleopold changed the title Update error enums 🚨 Wrap large enum variants in Box Oct 19, 2024
@JMicheli JMicheli merged commit 5608b5b into develop Oct 19, 2024
8 checks passed
@JMicheli JMicheli deleted the jm/update-enums branch October 19, 2024 00:50
@aaronleopold aaronleopold mentioned this pull request Dec 4, 2024
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.

2 participants