.info file and other 5.0 improvements

I think

  if (arg(3) && arg(4)) {
      //delete the fusion
      db_query("DELETE FROM {views_fusion} WHERE vid=%d AND mvid=%d", arg(3), arg(4));
      views_invalidate_cache();
      drupal_set_message('Your fusion has been deleted.');
      drupal_goto('admin/views/fusion');
     }

must be

  if (arg(4) && arg(5)) {
      //delete the fusion
      db_query("DELETE FROM {views_fusion} WHERE vid=%d AND mvid=%d", arg(4), arg(5));
      views_invalidate_cache();
      drupal_set_message('Your fusion has been deleted.');
      drupal_goto('admin/build/views/fusion');
     }

patch attached

The callback arguments in hook_menu() must be an array, otherwise I get an error on fused view deletion (but interestingly not on the normal fusion overview). Also, updating the README is always a good idea.

Apart from that, the patch seems to be ok, the module works as expected. Please apply and open a DRUPAL-5 branch.

Patch looks solid, I can't see why we need these dependencies checks though.

This patch goes ahead and removes them.

aren't the dependency checks in place to insure that views is installed and enabled before views fusion is enabled ?

In 5 the dependency system will take care of this as far as this module is concerned. That should be taken care of by moshe's .info file. The only time there would be a problem where a module requires another module during its install hook but this is so rare I'm not sure anyone other than ECommerce has even noticed. ;P Not relevant to this module.

With the .info file, we can ensure that both Views and Nodefamily are enabled when Views Fusion is.

The check in views_fusion.module does not check on Nodefamily being available. It makes sure that there is at least one possible relationship that fused views can be built on. Of course, Nodefamily provides that relationship, but this is not written in stone. While it seems unprobable that views_fusion support in Nodefamily will be removed, it could definitely happen, for whatever reason.

I think it's not wrong to do defensive programming here, and leave the check in. Also, if other modules gain views_fusion support in the future then we may drop the Nodefamily dependency, and in that case the check is needed anyways.

my fault, i didn't realize the patch wasn't patching the .info file.

I'm not going to argue about 3 lines. The rest of the dependency stuff is not needed though so I would suggest either this patch with the $tables['fields'] check or my patch in #4 without it. Doesn't matter to me as much as getting a 5 branch going so I can start using it :)

no argument from me, I initally thought the patch was applied against the .info file. The title of the thread threw me off. Thus I thought the dependency checks being removed were being removed from the .info file and not from the previous patch. Which is what prompted my question.

again my apologies.

Right, hunmonk's module dependency check is definitely not needed anymore. I second neclimdul's views_fusion_5_update_0.patch, please append the missing newline though before committing.

thanks, committed.

I've removed the dependency of the nodefamily module from the .info file, as it should work without too. E.g. an integration to the CCK nodereference field would make sense.