- Issue created by @jurgenhaas
- πΊπΈUnited States phenaproxima Massachusetts
We could implement this by replacing the unconditional
\RuntimeException
with this:if ($config->isNew()) { return $settings['value'] ?? throw new \RuntimeException("The '$name' config object does not exist."); }
Nice one-line fix and easy to test. I'd be okay with making this allowance for the
config
value source in particular. - @thejimbirch opened merge request.
- πΊπΈUnited States thejimbirch Cape Cod, Massachusetts
I created a MR with the change. Added Needs tests tag is someone wants to update/add a test for this.
- π©πͺGermany jurgenhaas Gottmadingen
Ready for a review, the nightwatch failure doesn't seem to be related.
- πΊπΈUnited States phenaproxima Massachusetts
This looks good to me but I don't think we need to bring the console-specific input collection stuff into it.
- πΊπΈUnited States thejimbirch Cape Cod, Massachusetts
Updated issue summary.
- πΊπΈUnited States thejimbirch Cape Cod, Massachusetts
We're going to park this for @alexpott to review.
- π¬π§United Kingdom alexpott πͺπΊπ
I think we also should support the fallback value for env too. The differences between the env and the config implementations are interesting. Env with return an empty string when the env var does not exist but config will throw an exception. I wonder what the right the thing to do here is...
- π¬π§United Kingdom alexpott πͺπΊπ
FWIW the concept of a fallback value seems go to me.
- πΊπΈUnited States phenaproxima Massachusetts
I like the idea of making it consistent.
- πΊπΈUnited States thejimbirch Cape Cod, Massachusetts
Looks great, Love the additional test and the comment updates.
I added a draft change record and am moving to RTBC.
- π¬π§United Kingdom alexpott πͺπΊπ
I think we should allow NULL as a fallback value. See review on MR.