Skip to content

refactor(core): remove dead code from Horde_Core_Ui_VarRenderer::factory()#101

Open
amulet1 wants to merge 1 commit into
horde:FRAMEWORK_6_0from
horde6:clean_factory
Open

refactor(core): remove dead code from Horde_Core_Ui_VarRenderer::factory()#101
amulet1 wants to merge 1 commit into
horde:FRAMEWORK_6_0from
horde6:clean_factory

Conversation

@amulet1
Copy link
Copy Markdown
Member

@amulet1 amulet1 commented May 12, 2026

No description provided.

@amulet1 amulet1 requested review from TDannhauer and ralflang May 12, 2026 20:47
{
if (is_array($driver)) {
$app = $driver[0];
$app = Horde_String::ucfirst($driver[0]);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new use of PSR-0 Horde_String - use the PSR-4 HordeString class.

Comment thread lib/Horde/Core/Ui/VarRenderer.php
}

if (!$ok) {
if (!class_exists($class)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine - we should not try to play autoloader.

@ralflang ralflang changed the title Remove no longer needed code from Horde_Core_Ui_VarRenderer::factory() refactor(core): remove dead code from Horde_Core_Ui_VarRenderer::factory() May 22, 2026
@TDannhauer
Copy link
Copy Markdown
Contributor

I looked into it based on the latest code in horde:

I agree that the old fallback/manual include path should not remain long-term. However, this PR is stale against current FRAMEWORK_6_0, which already contains a safer implementation using HordeString, app-name normalization, and a core renderer fallback. In RC phase I do not think we should merge a factory change that narrows runtime fallback behavior without tests across app renderers. Please rebase, update to latest horde code and add focused tests for scalar core renderer, lowercase app renderer names, missing app renderer behavior, and Composer autoload resolution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants