search

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
719 stars 820 forks source link

FIX Ensure environment is checked before enabling deprecations #11055

Closed GuySartorelli closed 7 months ago

GuySartorelli commented 7 months ago

Issue

GuySartorelli commented 7 months ago

I think I've still got it wrong - I'm gonna add checking the static currentlyEnabled to the test. Marking as draft in the meantime.

GuySartorelli commented 7 months ago

There we go, this should be correct and tests all scenarios.

GuySartorelli commented 7 months ago

And one last change to make the relationship between the static and the env variable clearer. This is ready for review now.

michalkleiner commented 7 months ago

It's quite hard to read compared to what it was before, but I think we need both the nested and the elseif stanzas.

  1. if env var set and explicitly disabled -> disabled
  2. if env var set and not disabled -> disable for non-dev, otherwise allow
  3. if env var NOT set, and not enabled via static prop -> disabled
  4. if env var NOT set, and explicitly enabled via static prop -> still disable for non-dev, otherwise allow
GuySartorelli commented 7 months ago

Yup, I must have forgotten to swap the target branch. Fixed and force-pushed to kick off tests again.