[TYPO3-core] Review system woes

[Dmitry Dulepov]

Hi!

Ernesto Baschny wrote:
> Dmitry, Helmut was so kind as to review your patch, give you feedback
> where he had doubts about if (see his review on 21st of December) and
> then even rebased your patch to the current master at the time the
> namespaces were introduced.
>
> So nothing "easy" about Helmut's -1, it has been thought and was only
> meant to give you an additional hint that the patch still needs some
> explanations.

This is what every core team member is supposed to do: review patches, 
improve code, add tests, etc. I am thankful to Helmut for rebasement but I 
think it is normal and it does not need extra mentioning, flowers, etc. I 
also do that for patches of others. I am not asking to be loved or get 
benefits for this and I do not think anybody should get extra recognition 
just because he does his work well. This is our responsibility as a core 
team members after all.

However being a core team member does not entitle to a -1 in case if that 
member does not understand what the patch does. Otherwise it just blocks a 
good patch because of misunderstanding. This is what happened here.

> Instead of investing your energy in putting down Helmuts effort you
> could simply explain how this is the solution to a problem which
> apparently Helmut was not able to reproduce. If this patch "works for
> more than 6 months on one of your sites" is a good indicative, but still
> not enough to include it in the core. That's why we have a review
> system, we cannot trust that simply because it "works for you" it will
> "work for everyone" (or not have any side effect).

It is very easy:

http://typo3.org/?cHash=bogus

There you get a non-cached page. Now imagine somebody putting that to an 
HTML page and waiting until Google indexes it and starts calling your pages 
with that regularly. That's the problem we had, which made a huge site very 
slow a couple of times a day for hundreds of pages. This is what the patch 
solves. I thought it is obvious from the code. No?

Do you still think it is a bad patch?

> Helmut's main point was: "If there are other possibilities where a cHash
> is appended in the core, it should be fixed there", as the methods
> Helmut examined were not susceptible to this problem. And thus the -1 is
> a just a response to your lack of further feedback.

No, it is not my lack of explanation. I already wrote in the first comment 
in Gerrit as a reply to Oliver: it is not about the core, it is about 
external requests. cHash in the core will be calculated correctly and will 
not do any harm. It is lack of people's wish to read explanations.

Again: why would I bother next time to send a fix to TYPO3?

-- 
Dmitry Dulepov
TYPO3 CMS core & security teams member

Simplicity will save the world.