fix(appinfo): properly type info parser and app manager for appinfo.xml types#60066
fix(appinfo): properly type info parser and app manager for appinfo.xml types#60066susnux wants to merge 4 commits into
Conversation
susnux
force-pushed
the
fix/type-app-info
branch
2 times, most recently
from
02a08c0 to
e3044a3
Compare
susnux
requested review from
Altahrim,
CarlSchwan,
leftybournes,
provokateurin and
salmart-dev
and removed request for
a team
provokateurin
left a comment
provokateurin
left a comment
There was a problem hiding this comment.
I'm a bit surprised by the behavior changes in parsing. Was it incorrect according to the schema?
| if (isset($array['collaboration']['plugins']['plugin']) && is_array($array['collaboration']['plugins']['plugin'])) { | ||
| $array['collaboration']['plugins'] = $array['collaboration']['plugins']['plugin']; | ||
| } |
There was a problem hiding this comment.
@provokateurin here the old code does nothing there are no such elements. The new code fixes a problem where wrong elements were parsed. This is similar to all other parsing of such elements in this function, e.g. see the if block before this comment.
| if (!array_key_exists('description', $array)) { | ||
| $array['description'] = ''; |
There was a problem hiding this comment.
@provokateurin there is no info inside the info, the <info> is the outer element - thats just always empty and unused.
The new code is just moved from applyL10N because there it made no sense but file sanitizing belongs to the parsing.
| /** | ||
| * @var AppInfoXmlDefinition $array | ||
| */ |
There was a problem hiding this comment.
This @var should not be there. To do it correctly you'd add all the checks that are needed to ensure the data matches the shape.
While I would like to see this, it's probably a bit overkill for this PR 🙈
There was a problem hiding this comment.
Lets do this in a follow up, I think it makes sense but not in the scope of this PR
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
4 participants
Summary
Checklist
3. to review, feature component)stable32)AI (if applicable)