[TYPO3-core] RFC #9474: Integrate OpenID authentication support to TYPO3

Ingo Renner ingo at typo3.org
Sun Oct 19 23:51:45 CEST 2008


Hi Dmitry,


here's my review:

line 34:
please use names for the variables that tell what information they 
contain, even if that information is not needed at this point.

line 47:
why do you need to loop over the array to find an empty spot? How about 
a simple count()

line 109:
please do not add commented code to the core. BTW: Why do you need that 
special function to add the field instead of simply using the core 
functions?

line 278:
smal typo: is -> if

line 279:
...there _is_ no...

line 344:
is it on purpose that there's nothong similar for the BE there?

line 16725, 16726:
typo: t3lub -> t3lib, decided -> decide

line 16728ff:
abbreviation: $authentificationInformation

line 16743ff, 16785ff:
abbreviation: $parentObject
(except for these two abbreviations I like that you used good variable 
names accros the code!)

line 16759:
$available - what is available? milk at the local grocery store?
Can you find a better, more descriptive name? Maybe $openIdAvailable 
will do?!

line 16799:
...returns _the_ user... back to -the- t3lib_userAuth...

line 16846:
$result -> $authenticated or even better $authenticationStatus

line 16896:
wouldn't be better to set the flag _after_ everything has actually been 
inlcuded?

line 16898:
incomplete comment?

line 16902:
you're using PATH_SEPARATOR around that line, why not in front of '/Auth'?

line 16942:
I'd suggest to call the method getUserRecord()

line 16966:
What about this TODO item, from the text it seems quit important to me?!

line 17034, 17070:
move the else one line up

line 17213:
not needed as it's already in the dependencies section further down. If 
someone's going to use this with a version lower than 4.3 it won't work 
anyways as the HTTP stauts constants in t3lib_div have been used...



so all in all I'd say 98% of the patch is great! Just fix these small 
remarks and we can get this baby in at last.


best
Ingo

-- 
Ingo Renner
TYPO3 Core Developer, Release Manager TYPO3 4.2


More information about the TYPO3-team-core mailing list