search

pat / combustion

Simple, elegant testing for Rails Engines
MIT License
707 stars 51 forks source link

Load defaults for Rails 7.2+ #140

Closed ankane closed 2 weeks ago

ankane commented 3 months ago

For #138

load_defaults was added in Rails 5.1: https://github.com/rails/rails/pull/28469

Also, this might be considered a breaking change.

ankane commented 3 months ago

Updated to only apply to Rails 7.2+ for backwards compatibility (as suggested in #138).

Edit: Also, sorry for all the noise.

pat commented 3 months ago

No need to apologise, appreciate you working on this!

I wonder if it's worth releasing this starting from 7.2+ initially (so it's not a breaking change) to confirm things work well, but then consider using it for 5.1+ as a major release/breaking change, and then that opens up removal of a couple of conditionals:

Not as many as I'd hoped, though! But still, I think it's worth the breaking change, as verifying we're using default behaviour in each release is a Very Good Idea.

ankane commented 3 months ago

Sounds like a plan. fwiw, I've been doing this for a few months for my projects and haven't seen any issues (as one data point).

pat commented 2 weeks ago

Finally published this in v1.5.0 - thanks for your patience (and contributions!) ❤️

ankane commented 2 weeks ago

Great, thanks @pat!