FIX Prevent invalid i18n sources from being collected #11603
Conversation
emteknetnz
force-pushed
the
pulls/5/fix-i18n
branch
from
be2f9e0 to
fad032a
Compare
There was a problem hiding this comment.
There were lots of invalid CLASS_DESCRIPTION: null being collected in multiple modules
This might point at us missing some configuration - totally fine if that's investigated in a separate card but at a minimum can you please either create a card to look into it, or list them here so I can look into it quickly before accepting this change?
| if (!empty($currentEntity[0])) { | ||
| if (!empty($currentEntity[0]) && !str_ends_with($currentEntity[0], '.')) { |
There was a problem hiding this comment.
I assume this is for
In assets File.php, an invalid key ending with "." with a value of "generic" was being collected
- Shouldn't we have a PR in silverstripe/assets to fix that key?
- Should we throw a warning when we find these instead of just ignoring them?
- We should update our PHPStan rule to capture this scenario so it doesn't happen again
There was a problem hiding this comment.
I don't think we should do an of those - here's the unit test that would trigger this before this PR https://github.com/silverstripe/silverstripe-framework/pull/11603/files#diff-7b72a58f2b9e604294d0342910391de6700b4c6fe15f99615bbaa729c196135dR1039 - it's a weird condition
There may be well other conditions in the future that mean we end up with a dot at the end of the key. I think having a catch-all "key ends with a period, it's clearly invalid, so just ignore it" is a pretty good real-world solution
There was a problem hiding this comment.
Discussed offline. We'll catch in PHPStan with silverstripe/silverstripe-standards#16 later on
|
New issue for missing class_description config #11607 |
Issue silverstripe/.github#365
Fixes a couple of bugs that showed up when running the text collector:
CLASS_DESCRIPTION: nullbeing collected in multiple modules