trungonly [trungonly]

that is great module! I thing it is very usefull for all people used ubercart!
Thanks!

Hello, and thank you for applying for a CVS account. I am adding the review tags, and some volunteers will review the code, pointing out what it needs to be changed.

As per requirements, the motivation message should be expanded to contain more features of the proposed project. For themes, it should include also a screenshot of the theme, and (when possible) a link to a working demo site; for modules, it should include also a comparison with the existing solutions.
What is the difference with http://drupal.org/project/attribute?

• The points reported in this review are not in order of importance / relevance.
• Most of the times I report the code that present an issue. In such cases, the same error can be present in other parts of the code; the fact I don't report the same issue more than once doesn't mean the same issue is not present in different places.
• Not all the reported points are application blockers; some of the points I report are simple suggestions to who applies for a CVS account. For a list of what is considered a blocker for the application approval, see CVS applications review, what to expect. Keep in mind the list is still under construction, and can be changed to adapt it to what has been found out during code review, or to make the list clearer to who applies for a CVS account.

1. License files cannot be committed in Drupal.org repository. Projects committed in Drupal.org repository have the same license used by Drupal.
2. Menu descriptions and titles, as well as schema descriptions, should not be passed to t().
3. See http://drupal.org/coding-standards to understand how a module should be written. In particular, see how Drupal variables, global variables, constants, and functions defined from the module should be named; how the code should be formatted.

Thanks a lot Trung for the very quick update.

The warning message disappeared from the Product CCK Attribute tab.
However, I now see the same error message when I add a product containing a CCK Attribute to the ubercart cart.

Warning message: "warning: Parameter 1 to uc_attribute_cck_add_to_cart_data() expected to be a reference, value given in .../includes/module.inc on line 483"
and "warning: Parameter 1 to uc_attribute_cck_product_description() expected to be a reference, value given in .../includes/module.inc on line 483"

Would really appreciate it if you can resolve that.

Thanks Trung,
Fadi

Trung, one more very useful feature would be the option to make the Product CCK Attribute required.
Thanks again,
Fadi

My bad, like you mentioned checking the "required" option on the CCK field configuration works....

The updated version you uploaded in post #10 resolved the last of my warnings.
Thanks Trung... great work.

Hey Trung,
One final modification would make this module super useful IMHO - If you can make the product summary (in the cart) display the CCK Attribute value.
For example, if I selected product "X" and attribute value "Y", it would be great if I can see it as "X(Y)" in the ubercart cart.

Do you agree?

Thank you,
Fadi

Thanks Trung.
It's working now. Unfortunately I'm not sure what I was doing wrong, but I believe it's probably related to the display settings of the field I wanted to display.
Thanks again,
Fadi

Here's another recommended feature: exposing the custom attribute stored through a CCK field attribute to the view module...
This would enable module users to create a view displaying the orders along with the custom CCK field(s) value(s).

I have used the "Ubercart Views" module (http://drupal.org/project/uc_views) to expose UC_Order fields... but the "data" field where you're storing the information (as far as I can tell) is not exposed.
I'm guessing that's because the data field by design is maybe meant to contain unstructured data that the use decides to capture with the order...

Thanks again for an incredible module...
Fadi

Great work Trung.
I am now able to use the CCK fields in a view.

Two comments: It would be great if the user can specify which of the multiple CCK fields I want to show in a given column field (camper name vs camper age in the attached screenshot).
It would also be great if the user can filter/sort by a given CCK field.

Thanks Trung,
Fadi

project = "ubercart"

That line needs to be removed as it is already added from the packaging script, and it would cause problems to the update manager that would not understand which one is the project name, and could not verify if there are updates for the module.

Thank you for your contribution!
I am going to update your account so you can opt into security advisory coverage now.
These 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 more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, 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.

I thank all the dedicated reviewers as well.

I am giving credits to the users who participated in this issue.