[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