[TYPO3-core] Discussion #27247 [BUGFIX] Bring back recStatInfoHooks hook in Page Tree

Jigal van Hemert jigal at xs4all.nl
Tue Jun 7 16:23:16 CEST 2011


Hi,

On 7-6-2011 15:54, Georg Ringer wrote:
> can we use this thread to discuss the issue
> http://forge.typo3.org/issues/27247
> https://review.typo3.org/#change,2546

Not specifically about this particular issue, but now we seem to have 
three locations for discussion. We used to have two (mantis before the 
RFC, core list when the RFC was made)...

> First of all:
> It's about a hook which has been there for ages and used by extensions
> (e.g. l10nmanager)
>
> A comment from gerrit by Jigal:
> ----------
> Reasons for -2:
> 1. Has this been checked with the UXW team? Maybe the hook was removed
> deliberately? Is it desirable to have extra visual clutter in the page
> tree?
> 2. The review process has been abused. Only the commit message was
> modified and now the original patch submitter has given a +2 (and no,
> it's certainly not a no-brainer to change functionality; even if it was
> present in an older version)
> ----------
>
> ad1: It hasn't been checked with the UXW team because IMO there is no
> reason for it because it is a hook and doesn't change anything in the
> output if the hook is not used. Therefore there can't be any visiual
> clutter or change.

The hook itself doesn't add anything to the page tree, but it's sole 
purpose is to add contents to the page tree items.

A good argument for legalizing the sales of drugs/weapons: if you don't 
use them they are not harmful ;-)

> ad2: Sorry if it seems that the review process has been abused. This is
> IMO not the case!

To make the picture clear:
1. person one pushes a change
2. person two adjusts the commit message and pushes change set 2
3. person one votes +2/+2

To me this is voting for your own patch.

> In my opinion this is a nobrainer because of the following reasons:
> * It is a hook and no other change

Hooks are not no-brainers. Each hook needs to be executed and thus 
requires processing power. This hook is processed for each element in 
the page tree, so it'll take quite some processing when building the tree.

> * and it has been there since a long time

Maybe it was removed for a reason? Code which has been removed cannot be 
re-introduced as a no-brainer just because it was there in a previous 
version.

> * The page tree has been implemented from the scratch and there were
> other things have been slipped through and still are (e.g. defalut
> values from TsConfig are not used when creating a page via page tree)

I can't see functionality which has been forgotten to implement as a 
no-brainer.

> PS: If you wonder why I want to have this hook so badly: Follow the
> discussion in the dev-list with title "Show changes for localization
> editors"

I have followed this discussion. The screenshot you produced there 
explains the reason why you want to have this hook. Even you must admit 
that the reason for wanting this hook is to be able to add extra 
information to the page tree. This invalidates your argument at ad1.

Frankly, the screenshot you provided [1] worries me a lot. During the 
development of 4.5 there has been a lot of discussion about what should 
be present in the pagetree and what not.
The current situation shows already too much for most editors to work 
with it comfortably. All the overlay icons are confusing and only show 
part of the settings.
There is a thread [2] discussing the page tree designs, but this died 
somehow. I also provided some ideas there [3] to make it less cluttered.

[1] http://img851.imageshack.us/img851/3664/l10nchangestree.gif
[2] http://forge.typo3.org/issues/9049
[3] http://forge.typo3.org/attachments/4027/pagetree_suggestion.png

-- 
Kind regards / met vriendelijke groet,

Jigal van Hemert.


More information about the TYPO3-team-core mailing list