Project

General

Profile

Actions

Bug #12396

closed

Review, CGL issues, security

Added by Ingo Renner over 13 years ago. Updated over 9 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Linkvalidator
Target version:
-
Start date:
2011-01-20
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
TYPO3 Version:
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

during a review I found some CGL and general coding style issues, sorry for the late review though...

Most of the stuff should be easy search and replace...

Blockers

They must be fixed until release.

=> See these subtasks:
#12410: Fix ChangeLog
#12411: Fix Copyright Notices
#12412: XSS issue in tx_linkvalidator_tasks_ValidateAdditionalFieldProvider
#12414: OOP principle of data encapsulation violated in tx_linkvalidator_tasks_Validate
#12416: tx_linkvalidator_tasks_Validate: Missing public / protected declarations

All blockers are fixed now.

The following things are no blockers.

UI and usabilty issues

  • why are there two buttons, one should be enough
  • the intro text at the top is nice when using the link checker the first time but should rather go into CSH instead of being there all the time
  • was the UI approved by the UI team?
  • remove the selection of (I guess) what links to show. Here, less is more. List all links independently of what checks have been hooked in.
  • use the rendered FE output to check for dead links instead of just checking the DB, this is more reliable (ok for 4.5, should be fixed in later versions)
  • might also use the reference index

=> Patrick explained those issues on the list.
For possible improvements see #12447.

General stuff

ext_tables.sql

=> Fixed in the subtasks
#12421: Rename tx_linkvalidator_links to tx_linkvalidator_link
#12422: Do not use abbreviations for field names (recuid, recpid)
#12423: Use underscores to separate words in field names

ext_localconf.php

See #12424

modfunc1

  • #12425: Rename modfunc1
  • #12426: Rename class names (naming conventions)
  • #12428: Naming of member properties: Use camelCase instead of underscores: $this->search_level
  • #12427: Rename draw*() methods to render*()
  • #12431: putting labels into members, is this necessary? (it's at least very uncommon): $this->firstSteps
  • #12429: Do not use private accessors
  • Not possible: why have initialize()? Would __construct() work?
  • #12647: No API used to build the links in renderTableRow()
  • #12648: the way the table is rendered does not necessarily follow TYPO3 API, check the scheduler for example. This is ok for 4.5 though, but should be changed later .

classes

See
  • #12426: Rename class names (naming conventions)

tx_linkvalidator_processing

  • #12649: analyzeRecord() nesting too deep, too long method / too many lines. try to split up (not required for 4.5)

tx_linkvalidator_tasks_Validate

  • #12650: exec() is too long (ok for 4.5)

Subtasks 5 (0 open5 closed)

Bug #12411: Fix Copyright NoticesClosedMichael Miousse2011-01-20

Actions
Bug #12414: OOP principle of data encapsulation violated in tx_linkvalidator_tasks_ValidateClosedMichael Miousse2011-01-20

Actions
Bug #12422: ext_tables.sql: Do not use abbreviations for field names (recuid, recpid)ClosedChris topher2011-01-21

Actions
Bug #12429: do not use private accessorsClosedMichael Miousse2011-01-21

Actions
Bug #12431: putting labels into members, is this necessary (it's at least very uncommon): $this->firstStepsClosedMichael Miousse2011-01-21

Actions
Actions #1

Updated by Ingo Renner over 13 years ago

classes

  • Naming: (see CGL) class names are lowercase namespace + uppercase noun class name

... UpperCamelCase noun class name

Actions #2

Updated by Patrick Gaumond over 13 years ago

  • Status changed from New to Accepted

Changed to Accepted but will need to be separate for better management.

Actions #3

Updated by Chris topher over 13 years ago

I created own issues for the blockers, which Ingo noted above.

They must be fixed...and they are fixed now.

And I also began creating issues for the other things.

Actions #4

Updated by Michael Miousse over 13 years ago

I have tested __construct() instead of initialize and it does not work
due to the context in whitch the module is called

Actions #5

Updated by Chris topher over 13 years ago

  • Status changed from Accepted to Resolved

Most of this has already been dealt with for TYPO3 4.5.

I have now created subissues for everything reported here.

We will go on there.

Actions #6

Updated by Chris topher about 12 years ago

  • Status changed from Resolved to Closed
Actions #7

Updated by Michael Stucki over 10 years ago

  • Category set to Linkvalidator
Actions #8

Updated by Michael Stucki over 10 years ago

  • Project changed from 1510 to TYPO3 Core
  • Category changed from Linkvalidator to Linkvalidator
Actions

Also available in: Atom PDF