Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Remove unnecessary require_once from codebase #803

Closed
robocoder opened this Issue · 17 comments

2 participants

@robocoder
Collaborator

In #620, we implemented an autoloader. In this ticket, we clean up the code by removing unnecessary require_once statements. This will require some analysis.

  • core/Piwik.php - add this as a special case to the autoloader or rename it to Helper.php (class Piwik_Helper)?
  • some files contain multiple class definitions, e.g., core/Auth.php contains Piwik_Auth and Piwik_Auth_Result
  • ./piwik.php - keep it as fast as possible; autoloading for a cache miss is ok

Don't change:

  • naming of "core" & "plugins" folders
  • FrontController.php and PluginsManager.php - they have their own autoloaders; don't change this
@robocoder
Collaborator
  • obviously, where we require_once a file that doesn't contain a class, keep it, but turn it into an absolute path (i.e., prefix it with PIWIK_INCLUDE_PATH)
@robocoder
Collaborator
  • don't remove the require_once from Zend folder until it has been updated (#497)
@robocoder
Collaborator

(In [1224]) refs #620, refs #803 - refactor autoloader into core/Loader.php (class
Piwik_Loader); remove unnecessary require_once in core/Common.php

@robocoder
Collaborator

(In [1225]) refs #803 - add autoloader to tests

@mattab
Owner
  • concerning when two classes are in the same file, are you planning to lookup the class name without last part (to look core/Auth.php for the Auth_Result class) or do you plan to create the new file?
@robocoder
Collaborator

My thought is to leave subclassed Exceptions where they are. We can add code to the autoloader to handle this, assuming I got the renaming right.

Something like Auth_Result should be in its own file for speed and consistency. An alternative is to continue to use require_once for these cases.

@robocoder
Collaborator

Actually, Piwik_Auth_Result isn't a good example as it and Piwik_Auth are currently always used in the same context. (core/Access.php, core/Auth.php, plugins/Login/Auth.php)

@robocoder
Collaborator

(In [1228]) refs #803 - auto-discovery algorithm to handle class Piwik, files with
multiple class definitions (e.g., Piwik_Auth_Result), and fix typo
(reference to Piwik_Api_Proxy).

@robocoder
Collaborator

(In [1229]) refs #803 - temp fix: PluginsManager defines functions in global scope

@robocoder
Collaborator

(In [1231]) refs #803 - temp fix; Translate.php contains global scope functions

@robocoder
Collaborator

(In [1232]) refs #803 - Log/Exception contains classes that won't be found by
autoloader's discovery algorithm; FrontController.php and Common.php
use classes with global scope functions (i.e., this needs to be
refactored)

@robocoder
Collaborator

(In [1233]) refs #803 - Auth is loaded by piwik.php?

@robocoder
Collaborator

Ok, files containing global scope functions will continue to be manually included (via require_once).

@robocoder
Collaborator

(In [1248]) refs #803 comment:11 - rename Log Formatter classes for consistency

@mattab
Owner

increasing priority - now that the loader is in place, maybe we should aim to close the ticket and remove all require_once - vipsoft if you disagree please reset the milestone

@robocoder
Collaborator

(In [1296]) fixes #803 - remove unnecessary require_once from core, plugins, and parts of
libs. (I didn't touch: open-flash-chart, Zend Framework, and PEAR HTML.)

@robocoder
Collaborator

(In [1299]) refs #803, refs #735 - lower default to 200; remove unnecessary require_once; standardize setting of include_path

@robocoder robocoder added this to the Piwik 0.4.2 milestone
@robocoder robocoder self-assigned this
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.