[TYPO3-core] RFC #11431: Hook request for manipulating the pageID in getPage

Oliver Hader oliver at typo3.org
Sun Aug 30 15:36:54 CEST 2009


Hi Christian,

Christian Kuhn schrieb:
> Hey,
> 
> Oliver Hader wrote:
>> Oliver Hader schrieb:
>>>> I hope this will lead to the missing +1 on testing from a core dev ;)
>>> Thanks for providing a unit test that looks good so far.
>>> Thus: +1 on testing
>>>
>>> However, I have some remarks concerning the patch:
>>> 1) it's not said that the hook is only used for manipulating the page
>>> uid, other things could be checked, too
>>> -> I'd prefer something like getPage_preProcess instead of manipulatePageUid
>>> 2) If you call a specific method in the hook object, it should implement
>>> a given interface (e.g. look to tslib/interfaces/ to see how they are
>>> used) - otherwise you could also use the callUserFunc alternative that
>>> allows one to define the method to be called
>> I forgot one thing:
>> Instead of testing with
>> | $this->assertTrue(!$variable)
>> you shout better use
>> | $this->assertFalse($variable)
> 
> 
> Thanks Oliver. Attached another version that honors all your points. I
> also moved the hook to the very top of the method.
> 
> As this version again changed a bit I would like to get a new review
> from anyone.

The patch is fine now, the behaviour did no changed it was just
rearranged and refactored. But since you provide an overworked unit test
for that it's easy to test that feature.

I'd say go ahead and commit it to SVN Trunk.

olly
-- 
Oliver Hader
TYPO3 Release Manager 4.3


More information about the TYPO3-team-core mailing list