Function names must be prefixed by the name of the module which defines them to avoid conflict with other modules; that is also true for functions whose name starts with an underscore.

CommentFileSizeAuthor
#14 kaltura-1009044-theme_advanced_editor_not_prefixed.patch812 bytesxurizaemon
#13 kaltura-1009044-theme_advanced_editor_not_prefixed.patch0 bytesxurizaemon
#12 kaltura-352670-function-name-prefixes-DRUPAL-6--2_v2.patch4.46 KBxurizaemon
#11 kaltura-352670-function-name-prefixes-DRUPAL-6--2.patch9.46 KBxurizaemon
#10 352670-function-name-prefixes-DRUPAL-6--2.patch9.74 KBxurizaemon

Comments

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

The only exception is for theme functions, of course, whose name must start with theme_.
Still, to avoid conflicts, the theme functions defined by kaltura.module should have a name starting with theme_kaltura_.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Function names must be prefixed by the module name » Prefix the function names with the module name
Category: bug » task

This is not exactly a bug, but it can cause problems, and therefore it must be changed.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Category: task » bug

If it's not following the coding standards, it's definitely a bug, especially with something that could cause name collisions. I mean this in the best way possible, but from the number of issues in this module's queue for things that do not follow Drupal module coding/standards, it seems like the developers really need to take a look at core modules and the coding standards. There are lots of unacceptable bugs.

Zohar.Babin’s picture

Zohar.Babin commented

Kiam, David,

This bug/task has already been applied on the code, but yet to be committed to the CVS.
We do our best to fix all issues as fast as we can, as well as comply the coding standards.

Gonen

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Sounds good, just make sure to keep us updated in the issue statuses when each issue is fixed in CVS. Thanks gonen!

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Prefix the function names with the module name » Function names must be prefixed by the module name

If the category has been changed back, then also the title should be changed.

Zohar.Babin’s picture

Zohar.Babin commented
Status: Active » Closed (fixed)

all functions prefixed in 6.x-1.4 and 5.x-1.3

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
Version: 6.x-1.3 » 6.x-2.x-dev
Status: Closed (fixed) » Active

This issue was apparently never really resolved, or reappeared in a later checkin.

The following functions are incorrectly named as of current DRUPAL-6--2 and DRUPAL-6--1 - I haven't included here function names which will be renamed as a result of #877444: Module namespacing.

DRUPAL-6--2

  • kaltura.module
    • get_entry_description
    • katura_fetch_widget_html
    • katlura_get_metadata_for_mix
    • theme_list_of_entries
  • includes/kaltura.admin.inc
    • register_kaltura_partner
    • import_ce
    • alternate_xml_verification
  • includes/kaltura.themeing.inc
    • theme_contribution_wizard
    • theme_contribution_wizard_add
    • theme_contribution_wizard_simple
    • theme_contribution_wizard_field
    • theme_simple_editor
    • theme_advanced_editor
  • kaltura_client/kaltura_helpers.php
    • getSitePlaylistPlayers

DRUPAL-6--1

  • kaltura.module
    • katura_fetch_widget_html
  • includes/kaltura.admin.inc
    • register_kaltura_partner
    • import_ce
    • alternate_xml_verification
  • includes/kaltura.themeing.inc
    • theme_contribution_wizard
    • theme_contribution_wizard_add
    • theme_contribution_wizard_field
    • theme_simple_editor
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

They are a lot of functions. Thank you, grobot, to re-open this issue.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
StatusFileSize
new 352670-function-name-prefixes-DRUPAL-6--2.patch9.74 KB

For DRUPAL-6--2. Partially tested only. Probably requires an update hook to force clearing the theme cache, as we have theme functions which are renamed here.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
StatusFileSize
new kaltura-352670-function-name-prefixes-DRUPAL-6--2.patch9.46 KB

updated patch committed to DRUPAL-6--2 in http://drupal.org/cvs?commit=452164

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
StatusFileSize
new kaltura-352670-function-name-prefixes-DRUPAL-6--2_v2.patch4.46 KB

Additional to previous patch.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
StatusFileSize
new kaltura-1009044-theme_advanced_editor_not_prefixed.patch0 bytes

From #1009044: Fix for advanced editor theme

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
StatusFileSize
new kaltura-1009044-theme_advanced_editor_not_prefixed.patch812 bytes

'git diff' not so informative post-commit ... let's try this one :)

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
Issue tags: +Cleanup

In field_kaltura.module, function kaltura_is_kaltura_field() should be moved to kaltura.module or renamed to field_kaltura_is_kaltura_field() (which will be an awesome function name btw)

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue summary: View changes
Status: Active » Closed (outdated)

I am closing this issue since it's for a Drupal version that isn't supported.