As there are different branches of the project that could be installed on a web site (6.x-0, 6.x-1, 6.x-2), it could be useful for a third-party module to know which version of the API the current XML Sitemap modules support.

For example, it would be enough a function like

function xmlsitemap_api() {
  return "2.0";
}

Comments

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Function to make third-party module know which branch of the project is installed » Add a function that return the version of the API implemented from the modules

There is actually a module that integrates with XML sitemap, which could take vantage of this function: Gallery.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Add a function that return the version of the API implemented from the modules » Add a function that returns the version of the API implemented from the modules

There is a little grammar error in the title; or better, there was.

dave reid’s picture

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

This should actually be hook_xmlsitemap_api and therefore xmlsitemap_xmlsitemap_api() to prevent name collision. If we're going to do this, we should take it a step further and actually check the api version from modules when getting the sitemap links instead of just adding one simple hook.

dave reid’s picture

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

For example, instead of doing something like this:

$links = module_invoke_all('xmlsitemap_links');

We need to do this:

define('XMLSITEMAP_API', 2);

function xmlsitemap_get_modules() {
  static $modules;
  
  if (!isset($modules)) {
    $potential_modules = module_implements('xmlsitemap_links');
    foreach ($potential_modules as $module) {
      if (module_invoke($module, 'xmlsitemap_api') == XMLSITEMAP_API) {
        $modules[] = $module;
      }
    }
  }
  
  return $modules;
}

Is this really a necessary step for us to take? We're not as big and integrated with everything like Views, so why can't we just work with the module maintainers to update their implementations? Please validate.

dave reid’s picture

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

The API number should also be a number value and not a string so that we can compare 1 < 2 or 1.5 < 2, etc.

avpaderno’s picture

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

It could be also a string, and passed to version_compare().

avpaderno’s picture

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

I was thiking to something that would help a third-party modules to understand if the version of XML sitemap installed is the one they support; that is why I proposed a simple function.

The maintainers can change their code, but they cannot control the version of XML sitemap installed.

dave reid’s picture

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

The views API uses numbers, so that's the example I'm using.

See http://drupal.org/node/418750 and http://api.lullabot.com/hook_views_api

The way I think this should be implemented is the way Views does it where the xmlsitemap module would check the xmlsitemap_api versions of all the implementing modules like I have shown in #4. Otherwise I don't think this is useful to other module maintainers at all.

avpaderno’s picture

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

That is fine, for me. I just didn't want to give the impression of suggesting something too complicated, but something simple that works on the case of XML sitemap, which (as you remembered) doesn't need all the complexity of Views. My comparison with Views was because it's a well known example of a module that checks the other modules for the compatibility they have with it.

dave reid’s picture

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

Yeah, agreed. I'm just still not sure what the usefulness of the function is and how it can force other module developers to work with the proper APIs. Plus, this function has been removed from the 6.x-1.x branch.

avpaderno’s picture

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

I removed it until we don't agree on something.
My idea was that third-party modules should verify the value returned from the function. The problem is how to use the function, which would not avoid a custom hook is invoked when it is not the right hook for the currently installed version of XML sitemap.

I can think of just an alternative for this problem.

avpaderno’s picture

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

The only alternative I can think of is that the code verify if the modules implement two different custom hooks.

I explain myself better; rather than calling all the implementations of hook_xmlsitemap_links(), which is the cause of not compatibility, the code should call the implementation of hook_xmlsitemap_links() for all the modules that implement also hook_xmlsitemap_xyz(). hook_xmlsitemap_xyz() should be a custom hook that is already used, but it's only required for a specific version, or a custom hook that the particular branch will ask to third-party modules to implement for compatibility.
In the later case, the custom hook must not do anything, nor return anything; it should be just a place holder that the code will check for, without to using any value returned by the custom hook.

This solution should at least avoid that a third-party thought to be used from a particular XML sitemap branch will be used from a different branch.

avpaderno’s picture

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

I looked at Chaos Tools, and it implements a function that returns the actual API version implemented by the module.

The comment placed in the module clearly states:

/**
 * Test the CTools API version.
 *
 * This function can always be used to safely test if CTools has the minimum
 * API version that your module can use. It can also try to protect you from
 * running if the CTools API version is too new, but if you do that you need
 * to be very quick about watching CTools API releases and release new versions
 * of your software as soon as the new release is made, or people might end up
 * updating CTools and having your module shut down without any recourse.
 *
 * It is recommended that every hook of your module that might use CTools or
 * might lead to a use of CTools be guarded like this:
 *
 * @code
 * if (!module_invoke('ctools', 'api_version', '1.0')) {
 *   return;
 * }
 * @endcode
 *
 */

EDIT: the author of that module is the same who created Views.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Postponed

I'm still not sold on this at all. :/

Anonymous’s picture

Anonymous (not verified) commented

I think it's unneeded fluff that someone maybe will use. We can always add it if some module maintainer using the xmlsitemap API wants it. I just don't see that need.

avpaderno’s picture

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

It would be unneeded fluff if you would be able to force any people to use the same branch of the project; differently, third-party modules would have to deal with the current branch installed from the users. As already there is a case, that is not an abstract consideration.

I don't understand what the concept of implementing something only if required is; does that mean a bug is fixed only if somebody opens a bug report?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Postponed » Closed (won't fix)

Won't fixing this issue for now. We can revisit this if we have lots of modules that implement the APIs, which is not the case right now.