[dev] Constructive Criticism/Venting

[Michael M Slusarz]

Quoting Michael J Rubinsky <mrubinsk at horde.org>:

> Quoting Michael M Slusarz <slusarz at horde.org>:
>
>> Just a random e-mail to share some thoughts.  Some come from  
>> experience refactoring several things over the last month or so.   
>> Some come from reading e-mails and bug reports.
>>
>> * Horde_Core a a package is completely unnecessary.  It really  
>> makes no sense separate from the horde application.  For H6 I see  
>> no reason not to combine the two (they are identical to upgrade.)
>
> I disagree. It allows a fix/update to Core functionality to be  
> pushed without requiring a full fledged Horde release.

Except this is a falsity.  i.e. you add a configuration option to  
horde and it doesn't do anything because the code that uses it is in a  
separate library?  That makes little/no sense.

And why should it matter that you are upgrading an application vs. a  
library?  From the PEAR perspective, they are identical actions.  And  
from an admin perspective there is no difference either (within point  
releases, you should be able to upgrade without worrying about  
configuration upgrades).

Especially since Horde_Core REQUIRES configuration changes as it  
currently stands - which makes it behave differently than every other  
library we distribute.

Horde_Core is worthless without horde and vice versa.  We don't gain  
anything from keeping them separate other than unneeded complexity.

>> * Dependency injection is useless IMHO.  We really aren't using it,  
>> so most/much of Horde_Injector functionality is unnecessary  
>> overhead.  E.g. as our AJAX processes become smaller, the  
>> Horde_Injector overhead because a greater part of the process  
>> runtime.  IIRC, it now takes up to 10%, for things we don't use -  
>> like Reflection analysis on every access (maybe this can be  
>> optimized/cached).  For our (Horde applications) purposes,  
>> dependency injection isn't tremendously useful since we may have  
>> multiple configurations for the same object - i.e. we have  
>> different DB setups for different apps, so each DB object needs to  
>> be manually injected.  Not to mention it is almost impossible to  
>> try to trace/debug the code when using dependency injection.  I've  
>> personally spent hours making small changes in Horde components  
>> code, simply because it can be impossible to try to trace where the  
>> object is coming from.  Horde_Injector is nice as a way to provide  
>> a global singleton/configuration store, but that's about it.
>
> I mostly agree, we have so many backends/configuration options that  
> using a pure DI solution (as opposed to what we do, and use it  
> mostly as a container for singletons) doesn't really work for us.  
> There are a few places, however, in the code where we *do* use the  
> injector's dependency resolver, so we have to be careful about  
> removing any of that functionality. That being said, just because we  
> are not currently using it doesn't mean we should abandon it. We  
> want other developers to use our libraries, so we shouldn't be  
> making this decision for them. I agree we could/should probably  
> refactor things so that stuff like the Reflection analysis doesn't  
> run on every page load.

I have no issue with keeping the injection parts of Horde_Injector for  
the code that does use it.  But we should also have a different  
framework dealing with the other 95% of our instantiations that don't  
need the extra features.

Maybe this is as easy as defining a new driver for Horde_Injector that  
skips all the Reflection stuff for classes that don't need it.

>> * We really need to split the git repo (I know, I know, I won't  
>> shut up about this).  Whether we like it or not, composer is the  
>> package manager of choice going forward (heck: Pirum is deprecated  
>> in favor of compose).  And I spent an afternoon trying to get  
>> composer to work with our current structure, and it simply doesn't  
>> work -- it requires both individual repos and specific tag naming.
>
> I've come to agree that this needs to be done, as I've said before.  
> However, we need to delay this until at least H5.2 is released. We  
> just don't have the manpower to perform the split, rewrite our  
> development tool chain, as well as work on the release/bug fixes  
> etc... I do, however, still adamantly stick to my view that we need  
> to have the tool chain developed and mostly working before we make  
> the official switch.
>
> Composer is another story. I have big concerns about moving to a  
> fully Composer managed system. If we do invest more heavily in  
> Composer, I think we need to still maintain the pear packages, at  
> least for one major release, to ease the pain of downstream  
> packagers - among other reasons.

See my previous email.  I have NO desire to give up PEAR.  But we need  
to better integrate composer rather than the half-hackish way we are  
doing it now.  We should not be counting on Pirum to serve composer  
configuration, because 1) it is no longer being supported and composer  
is still evolving and 2) it really doesn't work reliably when mixing  
with PEAR-based repositories, as is currently the case.

Packagist would allow us to distribute these libraries and provide  
another channel for both installation purposes and advertising  
purposes.  It is sad to see code on there that, in so many words,  
sucks compares to libraries we have written, yet has been downloaded  
10s of thousands of times.  Those are all potential  
customers/consumers that we are missing simply because we have not  
broadened our distribution channels enough.

michael

___________________________________
Michael Slusarz [slusarz at horde.org]