[D7] Commerce Attributes Date

I think there are some warnings generated from pareview which should be fixed.

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588

* 7.x-1.x-dev
remotes/origin/7.x-1.x-dev
remotes/origin/HEAD -> origin/7.x-1.x-dev

./commerce_attributes_date.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
function date_options_list($field, $instance, $entity_type, $entity) {

Coder Sniffer has found some issues with your code (please check the Drupal coding standards).

FILE: /var/www/drupal-7-pareview/pareview_temp/README.txt
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 10 WARNING(S) AFFECTING 10 LINE(S)
--------------------------------------------------------------------------------
24 | WARNING | Line exceeds 80 characters; contains 95 characters
37 | WARNING | Line exceeds 80 characters; contains 109 characters
42 | WARNING | Line exceeds 80 characters; contains 84 characters
43 | WARNING | Line exceeds 80 characters; contains 88 characters
47 | WARNING | Line exceeds 80 characters; contains 102 characters
49 | WARNING | Line exceeds 80 characters; contains 95 characters
50 | WARNING | Line exceeds 80 characters; contains 122 characters
56 | WARNING | Line exceeds 80 characters; contains 107 characters
57 | WARNING | Line exceeds 80 characters; contains 89 characters
65 | WARNING | Line exceeds 80 characters; contains 110 characters
--------------------------------------------------------------------------------

FILE: ...var/www/drupal-7-pareview/pareview_temp/commerce_attributes_date.module
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 4 WARNING(S) AFFECTING 4 LINE(S)
--------------------------------------------------------------------------------
41 | WARNING | Line exceeds 80 characters; contains 84 characters
46 | WARNING | Line exceeds 80 characters; contains 83 characters
100 | WARNING | Line exceeds 80 characters; contains 82 characters
104 | WARNING | Line exceeds 80 characters; contains 88 characters
--------------------------------------------------------------------------------

FILE: ...pal-7-pareview/pareview_temp/includes/commerce_attributes_date.date.inc
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
15 | WARNING | Line exceeds 80 characters; contains 107 characters
--------------------------------------------------------------------------------

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then 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.

1. add option configure to commerce_attributes_date.info

configure = admin/commerce/config/extra-attributes

2. when uninstall remove variable in includes/commerce_attributes_date.admin.inc
hook_uninstall(add commerce_attributes_date.install file)

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
The following git branches do not match the release branch pattern, you should remove/rename them. See http://drupal.org/node/1015226

* 7.x-1.x-dev
  remotes/origin/7.x-1.x-dev
  remotes/origin/HEAD -> origin/7.x-1.x-dev

Review of the 7.x-1.x-dev branch:

• ./commerce_attributes_date.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
  

  function date_options_list($field, $instance, $entity_type, $entity) {
  

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

FILE: /var/www/drupal-7-pareview/pareview_temp/README.txt
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 10 WARNING(S) AFFECTING 10 LINE(S)
--------------------------------------------------------------------------------
 24 | WARNING | Line exceeds 80 characters; contains 95 characters
 37 | WARNING | Line exceeds 80 characters; contains 109 characters
 42 | WARNING | Line exceeds 80 characters; contains 84 characters
 43 | WARNING | Line exceeds 80 characters; contains 88 characters
 47 | WARNING | Line exceeds 80 characters; contains 102 characters
 49 | WARNING | Line exceeds 80 characters; contains 95 characters
 50 | WARNING | Line exceeds 80 characters; contains 122 characters
 56 | WARNING | Line exceeds 80 characters; contains 107 characters
 57 | WARNING | Line exceeds 80 characters; contains 89 characters
 65 | WARNING | Line exceeds 80 characters; contains 110 characters
--------------------------------------------------------------------------------


FILE: ...var/www/drupal-7-pareview/pareview_temp/commerce_attributes_date.module
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 4 WARNING(S) AFFECTING 4 LINE(S)
--------------------------------------------------------------------------------
  41 | WARNING | Line exceeds 80 characters; contains 84 characters
  46 | WARNING | Line exceeds 80 characters; contains 83 characters
 100 | WARNING | Line exceeds 80 characters; contains 82 characters
 104 | WARNING | Line exceeds 80 characters; contains 88 characters
--------------------------------------------------------------------------------


FILE: ...pal-7-pareview/pareview_temp/includes/commerce_attributes_date.date.inc
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 15 | WARNING | Line exceeds 80 characters; contains 107 characters
--------------------------------------------------------------------------------

Source: http://pareview.sh/ - PAReview.sh online service

Following the instructions in the readme I created a date field on the product type, set it to be a select widget, set it to appear on the add to cart form, and once creating multiple products and referencing them on a product display I see the select box but no values inside it. Tried changing to radio to see if that helped and it didn't show. Any steps I'm missing in testing? If I can get it to show I'd set as RTBC as I think this is an interesting feature to have in the commerce world

Otherwise the code is clean in terms of coder module and coding standards (great use of dependencies as I was able to just install via drush easily).

Yes I've tried all the various settings within the product display/product variation I can think of. The install is just a straight install then did a drush en on your module with all the dependencies installed. The field settings on the admin page for the Date Attribute are as you recommend yet I still am getting a blank select box on my add to cart form I'll include snapshots in my next comment of all the relevant settings pages so you can inspect my setup and see if the issue is on my end or on the module (this way they're not part of the issue's included files).

Damn new issue system - looks like I have to add them directly to the issue itself. Here they are:

manual review:

1. commerce_attributes_date_form_alter(): if you are only targeting one form you should use hook_form_FORM_ID_alter() instead.
2. date_options_list(): this should have a comment that the hook is implemented on behalf of the date module.
3. _commerce_attributes_date_date_field_allowed_values(): the raw DB queries look wrong, so this will only work with field SQL storage. Why can't you load the product/node and use the field values from there?
4. commerce_attributes_date_admin_settings_form(): the module_exists() call is useless, since the date module will always be there because you have a dependency on it.

But that are not critical application blockers, so I guess this is RTBC. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

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

no objections for more than a week, so ...

Thanks for your contribution, yogeshchaugule8!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

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 stay 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.