search

pat / combustion

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

Set active_support cache format on Rails 7.1 #131

Closed mjankowski closed 1 year ago

mjankowski commented 1 year ago

Rails 7.1 adds a deprecation warning when the version is left as the default (6.1) value here. Rails 7.0 intoduced a new format, but did not issue a warning.

pat commented 1 year ago

Thanks for this patch - unsure why CI wouldn't run, but let's merge it in and then I can fix it if required :)

pat commented 1 year ago

Ah, I'd locked CI to use ubuntu-18 - everything's happy on ubuntu-latest. Thanks again!

mjankowski commented 1 year ago

Thanks for quick merge. Random aside here, and maybe a tip for anyone stumbling across this issue.

I'm using combustion on an internal gem currently only being used on a rails 7.1 (pre-release) app. I was able to get the rails 7.1 defaults loaded via...

Combustion.initialize! :all do
  config.load_defaults 7.1
end

I wonder if that setting should just be default called all the time by combustion? I think all non-EOL rails at this point have that available to set. I suspect that doing that as a general case would remove the need for the one-off special case deprecation wack-a-mole ... and if you're running combustion in an Appraisals/matrix/CI scenario, its probably always what you want.

pat commented 1 year ago

I like the idea of this, but I do worry about how such a change may impact those using the gem. Also, I have worked on Rails apps that keep that setting lower (e.g. using 6.0 defaults in a 7.0 app) to at least temporarily avoid some upgrading hassles. So it could be that making a presumption about the setting is not wise? 🤔

mjankowski commented 1 year ago

Yeah fair point -- might cause issues in those scenarios.

Added a wiki page - https://github.com/pat/combustion/wiki/Configuration

pat commented 1 year ago

Great initiative, thank you!