[D7] Ubercart Edit Cart Item

There are some errors reported by automated review tools, did you already check them? See http://ventral.org/pareview/httpgitdrupalorgsandboxlaborb1418732git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Hi,
Nice and simple module, here are some thoughts on the D7 version:

Please change the
package = Ubercart Extra
to
package = Ubercart extra
to put it in the same module group as other extra modules

I got the following error when hitting the edit field:
Notice: Undefined index: attributes in uc_cart_edit_form_alter() (line 65 of [path-to-drupal]/sites/all/modules/contrib/ubercart_edit_cart_item/uc_cart_edit.module).
This needs to be fixed.

Quick code review:

I don't get this condition:

  // In the Ubercart 3.x version the uc_product_add_to_cart_form
  // are part of the uc_cart_view_form. Continuing here would lead
  // to errors, so we will just return.
  if (arg(0) == 'cart') {
    return;
  }

Why is there a check for url "cart"?

Hi

Sorry, I meant
Ubercart - extra
- you know, to put it in the same group as other extra modules

Ok now after quick code inspection, I started reviewing the functionality - what I dont understand is who would use this? When I click edit, I get the normal product node view with save button, but what is saved? Is it working as expected? (see screenshots)
On the 2nd screenshot, what am I supposed to do? There are GET parameters containing node ID (which is already there in node/[nid]) and nothing to change or edit.
Please explain

Hi

It makes more sense to me now, thanks for explanation.

Further code inspection:

lines 21, 22:
from php doc (http://php.net/manual/en/function.strstr.php):
"If you only want to determine if a particular needle occurs within haystack, use the faster and less memory intensive function strpos() instead."

There is nothing else from my side for now.

This project is too short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed. However, we can promote this single project manually to a full project for you.

Otherwise looks good to me from a visual code review. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to ELC as he might have time to take a final look at this.

Sure, just make sure that you reference your existing projects in that application.

no objections for a week, so ...

Thanks for your contribution, labor b!

I promoted this project for you: https://drupal.org/project/uc_cart_edit

Now that this experimental project has been promoted, you'll need to update the URL of your remote repository or reclone it.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.