Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate redundant use of plugin getName() #1152

Closed
mattab opened this issue Feb 11, 2010 · 8 comments
Closed

Deprecate redundant use of plugin getName() #1152

mattab opened this issue Feb 11, 2010 · 8 comments
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Milestone

Comments

@mattab
Copy link
Member

mattab commented Feb 11, 2010

Must it be the same as the class name?
Is it used anywhere?

let's clarify this and/or remove it if not useful (if we display the class name only, we don't need a human readable plugin name)

@robocoder
Copy link
Contributor

getName() isn't the same as the class name, e.g., 'CoreHome' vs 'Piwik_CoreHome'

getName() is used in two places:

  • in core/PluginsManager.php where it is used in the exception message if installPlugin() fails
  • in core/View.php where it is assigned to currentPluginName and displayed (human readable) next to the Piwik logo (in plugins/CoreHome/templates/logo.tpl)

This is not used by CorePluginsAdmin where it displays the name of each plugin. Here it uses readPluginsDirectory().

Given the above, we could enforce this consistency by adding a check to CorePluginsAdmin/Controller.php's index() -- skip any plugins where these don't match up.

@robocoder
Copy link
Contributor

Oh, I see what you mean. Yes, it would seem that getName() and getClassName() duplicate each other.

Ok. Revised suggestion:

  • document that getName() is the human readable plugin name (may contain spaces, punctuation; similarity to the plugin name is encouraged; translateable?)
  • in CorePluginsAdmin/Controller.php -- use getName() instead of the plugin's directory name when displaying the list of plugins
  • add a check to ensure all the human readable plugin names are unique, and that getClassName() matches the plugin's directory name; should be sufficient to check this constraint at installation, updates, and when installing new plugins

@mattab
Copy link
Member Author

mattab commented Mar 20, 2010

Sounds good

@mattab
Copy link
Member Author

mattab commented Mar 20, 2010

actually, why do we need getClassName() if it must return the directory name? maybe we can get rid of it and parse it from the dir name.

@robocoder
Copy link
Contributor

In http://dev.piwik.org/trac/wiki/Plugins/HowToWritePlugin?action=diff&version=15 - refs #1152, remove reference to "name" in getInformation() array

@robocoder
Copy link
Contributor

(In [2263]) refs #1152 -deprecate "name" in getInformation() array;use getClassName() instead of getName(); remove unused getName()

@robocoder
Copy link
Contributor

(In [2264]) fixes #1152 - remove deprecated "name" from getInformation() array

@robocoder
Copy link
Contributor

(In [2272]) refs #1152 - add "final" keyword to getClassName() because we dont want subclasses (plugins) to redefine this; remove "name" from phpdocs

@mattab mattab added this to the Piwik 0.6.3 milestone Jul 8, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Projects
None yet
Development

No branches or pull requests

2 participants