Skip to content

Conversation

@mamhoff
Copy link
Contributor

@mamhoff mamhoff commented Dec 21, 2024

Setting line item comparison hooks in a config.to_prepare block will currently autoload the Spree::Order model (and subsequently any model referenced in that class definition), slowing down the boot process in development mode (when we want our app/ directory to be autoloaded.

This moves setting those hooks to Spree::Config, which is already loaded by the time initializers run. This allows us to move code out of config.to_prepare hooks, and stop the order class from being inadvertently eager loaded.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@mamhoff mamhoff requested a review from a team as a code owner December 21, 2024 16:05
@github-actions github-actions bot added changelog:solidus_core Changes to the solidus_core gem changelog:solidus_promotions Changes to the solidus_promotions gem labels Dec 21, 2024
@mamhoff mamhoff marked this pull request as draft January 9, 2025 11:18
@mamhoff mamhoff force-pushed the configure-line-item-comparison-hooks-in-config branch from ddb140c to 49c44a5 Compare February 10, 2025 13:34
@mamhoff mamhoff marked this pull request as ready for review February 10, 2025 13:34
@mamhoff mamhoff force-pushed the configure-line-item-comparison-hooks-in-config branch from 49c44a5 to 05d4196 Compare February 10, 2025 13:35
Setting line item comparison hooks in a `config.to_prepare` block will
currently autoload the Spree::Order model (and subsequently any model
referenced in that class definition), slowing down the boot process in
development mode (when we want our app/ directory to be autoloaded.

This moves setting those hooks to Spree::Config, which is already loaded
by the time initializers run. This allows us to move code out of
`config.to_prepare` hooks, and stop the order class from being
inadvertently eager loaded.
@mamhoff mamhoff force-pushed the configure-line-item-comparison-hooks-in-config branch from 05d4196 to 81bf6f6 Compare February 10, 2025 14:26
@codecov
Copy link

codecov bot commented Feb 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.68%. Comparing base (efe8b97) to head (81bf6f6).
Report is 29 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6050      +/-   ##
==========================================
+ Coverage   86.62%   88.68%   +2.06%     
==========================================
  Files         511      829     +318     
  Lines       11809    18012    +6203     
==========================================
+ Hits        10229    15974    +5745     
- Misses       1580     2038     +458     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Thanks

@tvdeyen tvdeyen merged commit 7205ff0 into solidusio:main Feb 14, 2025
21 checks passed
@tvdeyen tvdeyen added this to the 4.5 milestone Feb 14, 2025
DanielePalombo added a commit to DanielePalombo/solidus_virtual_gift_card that referenced this pull request Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:solidus_core Changes to the solidus_core gem changelog:solidus_promotions Changes to the solidus_promotions gem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants