uc_flatrate.install still references db_num_rows()

It is also used in hooks.php!

uc_catalog.install, uc_product.install and uc_flatrate.install also have instances of db_next_id which is not used in D6. I just spent four hours sorting out this mess.

I have to disagree with ohnobinki . It is not unlikely that these functions will get called.

If you are touting Ubercart as D6 compatible, you should make sure it is.

This code is correct. Those update functions are used for upgrading from one Drupal 5 version to a newer Drupal 5 version. They will never be called when you are using Drupal 6 unless you fail to properly upgrade to the most recent Ubercart 1.x Drupal 5 version before upgrading to Ubercart 2.x Drupal 6.

As I said in #955584: Investigate use of hook_update_last_removed() in .install files, if they will never be called (except on upgrade), they should be removed, and hook_last_update_removed() should be implemented. There is no point including upgrade functions from D5 if they crash when called; the installer should notify the user that they have to update to the latest D5 version instead, which is exactly what that hook does. That other issue has been bumped to D7; this should still be done in the D6 version.

By closing this as "works as designed" are you really saying that Ubercart should crash on D5 -> D6 upgrade if the user failed to notice there were previous database updates that were not applied?

In other words, your upgrade path breaks if someone is foolish enough to install D6 before upgrading their contributed modules? Recognizing that this might happen should be reason enough to change the code to prevent it.

In my case, it was a fresh install of Ubercart 2.X on a D6 site, which was working correctly, but still showing unapplied updates, and throwing fatal errors while I was trying to upgrade other modules.

Yes, I'm saying that if you don't follow the Drupal-required upgrade procedures, you may break things. This happens to be true of Drupal core as well as Drupal modules in general; it is not an Ubercart "bug".

Specifically, Drupal coding rules say you're not supposed to change update functions ever - even if the Drupal API has changed (although they may be removed when no longer needed, as is recommended by longwave in #955584: Investigate use of hook_update_last_removed() in .install files). That is why I marked this "as designed". There's nothing wrong with the code in question - Ubercart is doing this the Drupal-approved way.

@longwave: I believe your comment at http://drupal.org/node/955584#comment-3636056 says, in part, essentially the same thing ("as designed"). The other part, the issue of adding hook_last_update_removed(), is still open and can be discussed there. There, I outlined my fuzzy understanding about what needed to be done - no one has since commented on or corrected that view nor offered a patch.

@triskelion: If you had a truly fresh install, there's no way Drupal should be showing unapplied updates. Drupal never applies any update functions on initial install, it simply runs the hook_install() for each module and records the highest-numbered update function for future reference. Drupal will show that there are unapplied updates only when it finds higher-numbered update functions present in the code after the initial install. Seems to me you might have multiple copies (& different versions) of Ubercart sitting around somewhere under your root Drupal folder.

@TR: what is the point of spending time writing a patch, when there are already numerous patches awaiting review or that have been reviewed by others, but that you (as apparent sole maintainer of the 6.x branch) are simply ignoring?

http://drupal.org/project/issues/search/ubercart?status[]=8&status[]=14&version[]=6.x

Setting back to active, as I will consider writing a patch if I get a response to the above.

@longwave: Your link shows 126 "Needs Review", which is comparable in number to other Drupal modules that have a large code base. Being a volunteer, unpaid maintainer doesn't obligate me to work on each and every one of these issues, or even on any of them. If there's no one in the community who finds the "Needs Review" issues important enough to address, then chances are they're not going to get addressed. The issue queue is not my personal to-do list. I do what I can, when I can, given the constraints of my job and personal life. If you have spare cycles to donate, I suggest you ask for CVS privileges. Your name is on the (short) list I gave to Andy last year of people who I think should have CVS access to the Ubercart project, if they want, but I don't control that.

This cleanup has been done in other issues.