Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Prefer explicitly using serde_derive instead of the "derive" feature of serde #1329

Merged
merged 7 commits into from
May 14, 2024

Conversation

kevinheavey
Copy link

Problem

Using the "derive" feature of serde is bad for compile times as explained here serde-rs/serde#2584

Summary of Changes

  • Deactivates the "derive" feature of serde everywhere and replaces it with explicit use of serde_derive
  • Makes the serde_derive version match the serde version in the main workspace
  • Removes the serde dep entirely from programs/address-lookup-table

Copy link

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks good to me! Just some nits to add comments about the versioning

@kevinheavey kevinheavey force-pushed the prefer_serde_derive branch from c64d14c to e48189a Compare May 14, 2024 20:42
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 93.93939% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 82.8%. Comparing base (0214743) to head (e48189a).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #1329     +/-   ##
=========================================
- Coverage    82.8%    82.8%   -0.1%     
=========================================
  Files         867      867             
  Lines      368875   368873      -2     
=========================================
- Hits       305646   305642      -4     
- Misses      63229    63231      +2     

Copy link

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Thanks!

@joncinque joncinque merged commit 59e3eaa into anza-xyz:master May 14, 2024
50 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants