Bug #28994
General issues from reading the code
| Status: | New | Start date: | 2011-08-16 | |
|---|---|---|---|---|
| Priority: | Should have | Due date: | ||
| Assignee: | - | % Done: | 0% |
|
| Category: | - | |||
| Target version: | - | |||
| Votes: | 0 |
Description
Hi,
as Olly asked in mailinglist for checking the code, here is a short report. I try to keep the CGL infos low, those can be cleaned up laster.
- Directories "Hook" and "Task" should be called "Hooks" and "Tasks" like Classes, Resouces and Tests. Direcotry "Javascript" should be called "JavaScript"
- tx_staticfilecache_Task_AdditionalFieldProvider_DeleteDirtyPagesTaskneeds a hsc() at value="' . $taskInfo['maxEntries'] . '" to avoid XSS. or maybe even an intval if this is a time period
- Classes should begin with Tx_ instead of tx_
- use debug statements for debugging, this makes it easier to find those at the end, e.g. in tx_staticfilecache_Hook_FrontendHook there is an "echo".
- Copyright notice is missing in some files
- Remove not needed files like changelog, readme.txt
- Move icon files into Resources/Public/Images, Move locallang_db.xml into correct directory. xlf?
- ext_conf_template.txt: texts can be localized
- Hooks in ext_localconf.php: I always use instead of "['t3lib/class.t3lib_tcemain.php']['clearPageCacheEval'][]" something like "['t3lib/class.t3lib_tcemain.php']['clearPageCacheEval'][$_EXTKEY]" because it makes it possible to change the hook in a 3rd party extension very easily
- tx_staticfilecache_Hook_FrontendHook: any of the variables covered with t3lib_div::getIndPenv() ?
- just from reading: is the directory "typo3temp/tx_staticfilecache" created if it is missing?
- tx_Staticfilecache_ExtDirect_Server: besides code commenting and phpdocs, I guess TYPO3 got somewhere a translated string of yes/no
- TYPO3 cgl uses a ?> at the end of the file, see ext_tables.php
thanks for your work!
History
Updated by Oliver Hader almost 2 years ago
Georg Ringer wrote:
Hi,
as Olly asked in mailinglist for checking the code, here is a short report. I try to keep the CGL infos low, those can be cleaned up laster.
- Directories "Hook" and "Task" should be called "Hooks" and "Tasks" like Classes, Resouces and Tests. Direcotry "Javascript" should be called "JavaScript"
Okay, could be considered...
- tx_staticfilecache_Task_AdditionalFieldProvider_DeleteDirtyPagesTaskneeds a hsc() at value="' . $taskInfo['maxEntries'] . '" to avoid XSS. or maybe even an intval if this is a time period
Obvious, thanks!
- Classes should begin with Tx_ instead of tx_
Ok, since it's based on Extbase - however only the backend module utilizes Extbase, the rest is still "plain" TYPO3...
- use debug statements for debugging, this makes it easier to find those at the end, e.g. in tx_staticfilecache_Hook_FrontendHook there is an "echo". - Copyright notice is missing in some files - Remove not needed files like changelog, readme.txt - Move icon files into Resources/Public/Images, Move locallang_db.xml into correct directory. xlf? - ext_conf_template.txt: texts can be localized - Hooks in ext_localconf.php: I always use instead of "['t3lib/class.t3lib_tcemain.php']['clearPageCacheEval'][]" something like "['t3lib/class.t3lib_tcemain.php']['clearPageCacheEval'][$_EXTKEY]" because it makes it possible to change the hook in a 3rd party extension very easily - tx_staticfilecache_Hook_FrontendHook: any of the variables covered with t3lib_div::getIndPenv() ?
Most of these issues make sense.
- just from reading: is the directory "typo3temp/tx_staticfilecache" created if it is missing?
Yes, it is...
- tx_Staticfilecache_ExtDirect_Server: besides code commenting and phpdocs, I guess TYPO3 got somewhere a translated string of yes/no
Not done yet, however the l10n layer could be introduced here.
- TYPO3 cgl uses a ?> at the end of the file, see ext_tables.php
thanks for your work!
Thanks for your feedback and analysis!