This is a proposal for using the SiteCatalyst external library (https://github.com/RiotGames/sitecatalyst/) and separate the omniture-related code from the drupal module.

At the beginnign (using the patch provided) this would only serve to clean up the embedded javascript code that is present in omniture.module, leaving the module functionality untouched.

In the long term this could be a mean to add functionality and improvements to future versions.

The patch illustrates a simple switch to the library that wouldn't break current functionality. It's using hook_libraries_info and xautoload to detect and load the class file, and it provides a make file for easily downloading the lib using drush.

Let me know what you thing about this.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lucascaro’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, omniture-use_sitecatalyst_lib.patch, failed testing.

lucascaro’s picture

The fail seems to be due to the changed dependencies :/ testing locally to confirm.

lucascaro’s picture

Status: Needs work » Needs review
FileSize
8.78 KB

Here's a new version of the patch that adds support for the tracking image. With this change the tests pass locally. The test bot is expected to fail due to the dependencies added by this patch (xautoload and libraries).

Status: Needs review » Needs work

The last submitted patch, omniture-use_sitecatalyst_lib-1840850-4.patch, failed testing.

lucascaro’s picture

Adding 'dependencies' key to the test class as per #1670936: Testbot does not detect/download additional dependencies to have the testbot run these tests only when the dependencies are available.

lucascaro’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, omniture-use_sitecatalyst_lib-1840850-6.patch, failed testing.

bleen’s picture

@lucascaro ... I'm definitely not loving the idea of adding dependencies to this module. If I could see some immediate benefit (other than cleaner code) my opinion could probably be swayed. Even if you could layout some hypothetical improvements that would be made possible (or significantly easier) by using the external library that would be helpful. I'm just not seeing it...

lucascaro’s picture

Status: Needs work » Closed (won't fix)

Let's talk about this again once we have significant benefits in the library :)