Hey. Great work on the module; it is really extensive. I just wanted to bring put a few points about namespaces:

* Module Namespace: Any sub-modules should be named with prefix of the core module. This is not required, documented, and CCK does not follow it, but it is bad practice, as WebChick has reinforced in IRC for me. Specifically you are using the "openlayers" module namespace, which is taken: http://drupal.org/project/openlayers

* Function Namespace: All functions within a module should be prefixed with the module name (not just hook implementations).

I started this thread to get some documentation on this: http://groups.drupal.org/node/21725

CommentFileSizeAuthor
#4 fix_namespaces.patch1.22 KBtmcw
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jpulles’s picture

Hi zzolo,

So far using the openlayers namespace for the openlayers map client didn't give a problem because of the absence of an openlayers module release. Also, it is probably unlikely that people (other than you and me) would want to enable both at the same time.
Apart from that, you are right that the module name should (and will) be different; but I always liked the short openlayers name for it...

Regards,
John

jpulles’s picture

Assigned: Unassigned » jpulles

I noticed the dev release of the openlayers module. The effort needed to enable both packages seems very limited :-)

zzolo’s picture

@jpulles,

This has little to do with what stage of a release or even what the code does for the openlayers module. It was clearly a project before you started this one, and as noted above, it is horrible practice to do what you are doing here. Please do not make me bring this up with the infrastructure team. I don't expect you to change this right away, but I do expect this to change. If you really want, I can write a patch.

--
zzolo

tmcw’s picture

Category: task » bug
Priority: Normal » Critical
FileSize
1.22 KB

Here's a patch that simply renames the openlayers_ functions to mappingkit_openlayers. Given that there is a bug report that strongly suggests some negative interaction between OpenLayers and Mapping Kit, this is an important issue, and 'liking the namespace' is not going to cut it. I can roll a patch that changes the module namespace as well.

jpulles’s picture

Thanks for this patch, I will apply it later on.

Please note that the bug report you mention talks of an old mapping kit version (6x-1.4); since then, the negative interaction isn't there any more.

tmcw’s picture

Any progress here?

jpulles’s picture

Priority: Critical » Normal
Status: Active » Fixed

Patch is committed to dev version.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.