search

silverstripe / silverstripe-assets

Silverstripe Assets component
BSD 3-Clause "New" or "Revised" License
9 stars 65 forks source link

ENH Use the optimal file resolution strat by default #542

Closed GuySartorelli closed 1 year ago

GuySartorelli commented 1 year ago

This change reflects the config that installer was already overriding it with, so new projects were using this strategy anyway. It should just be the default outright.

Parent issue

GuySartorelli commented 1 year ago

We still need to test that can validate that fallback resolution strategy still work

Do we though? This is a fallback on a legacy strategy for a legacy setup. No new projects will be using this configuration anyway. Any projects which have been around since this was needed (and therefore use this strategy for protected files) will throw deprecation warnings when they upgrade to 4.13 (see https://github.com/silverstripe/silverstripe-framework/issues/10692).

I would argue it's more appropriate to remove that from the test - if it does work and people use it as a workaround then fine, we're not going out of our way to break it per se, but we shouldn't actively support legacy fallbacks IMO.

That said, after reading this if you still think option 2 is preferable I'll do that.

GuySartorelli commented 1 year ago

Nevermind, it turns out these tests are kinda complex and it's easiest to just add that config back in for them