[TYPO3-core] RFC: #16315: New extension manager part 1: splitting classes

Ernesto Baschny [cron IT] ernst at cron-it.de
Thu Nov 11 10:07:47 CET 2010


Hi Steffen,

Steffen Kamper schrieb am 11.11.2010 09:35:

> here is a new patch which
> * fixes things Xavier mentioned
> * fixed definition of task
> * fixed an issue with updating em_conf
> 
> Ernesto, before you do please have a look so that there is no double
> work, maybe your findings are already solved with this patch.

Thanks, mostly it is the case. I have noted that you unindented many
"//" comments that used to be indented. But that extra indention is
currently our CGL rule, so consider moving them back before committing.

Also reviewing this patch without having a clear insight on what exactly
was done is pretty difficult, especially since methods and functions
have been moved around files. This is difficult to spot by reading the
patch.

For example this diff was helpful:

OLD: typo3/sysext/em/mod1/class.em_index.php
NEW: typo3/sysext/em/classes/index.php

But then, why is the "old EM" moved to the refactored
"classes/index.php" when it is full blown still available in
mod1/class.em_index.php? I understand this is an intermediate step, but
then I would also have deleted mod1 completely, else this intermediate
step makes no sense.

Or even better, would have been refactor the "old module"
mod1/class.em_index.php in place to use the new classes.

Anyway, it works as far as I could test, the CGL and refactoring seem
not to have broken anything, and the new classes-layout appears to make
sense. Without seeing the "new EM" it is difficult to judge it further,
so I would +1 by reading and testing to get this one going so that we
have time to look also at the "new EM". :)

Thanks for the enormous amount of time and work put into this project!

Cheers,
Ernesto


More information about the TYPO3-team-core mailing list