Basic Conferences

Hi indydas, good module!

Although, there're some format issues. Please take a look at http://ventral.org/pareview/httpgitdrupalorgsandboxindydas1837860git

I notice that four content types are created when enabled to ensure this conference feature. Maybe it's better to check current site to make sure no such content type has been created before this module installed, i.e, a content type called 'room' has alreay been created in current site, and this module will try to create it again:)

In template file, like node-speaker.tpl.php, only specific fields value have been rendered. If users have added other fields to such conent types, users will need to rewrite these templates. From this point, this module is kind of only work for users with some technical background. Is there any chance that we make it more common to use?

hi,
For that array format issue, maybe you could try putting

    'allowed_values' => '10
    9
    7
    6
    5
    4
    3
    2
    1
    0
    -1
    -2
    -3
    -4
    -5
    -6
    -7
    -8
    -9
    -10',

available values in a single line, with commas~

Some times, if you're not sure about the format, you could try some possible different format to do this, or check the code standarding again:)

Project applications are tasks.

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

You need to add the tag "PAReview: review bonus" to show up on my high priority list.

Doing my review against a fresh install of Drupal 6.

There seems to be two modules missing from your module's dependecies list:
"Content Copy" and "Number"
The "Number" module was required during the install hook of the module, and threw an error then.

And there was an error complaining about a missing file in your module (referenced in hook_install):
includes/basic_conferences.session_time_install.inc

Also, the node_get_types function does not seem to do what you are expecting it to do (or maybe i'm missing something here?).
The api docs for node_get_types is here:
http://api.drupal.org/api/drupal/modules!node!node.module/function/node_get_types/6
Specifically, the first parameter should be one of: 'types', 'type', 'module', 'names', 'name'

You are also returning early in line 290 of your basic_conferences_uninstall function:

290:  return $ret;
291:
292:  // First check view exists and if it does delete conference view.
293:  if (views_get_view('conference')) {
294:    delete_view('conference');
295:  }
296:  else {
297:    drupal_set_message(t('Error', 'conference view does not exist'));
298:  }

lines after 290 are never reached.

A couple of small coding issues:

http://ventral.org/pareview/httpgitdrupalorgsandboxindydas1837860git

- On install, don't return FALSE if you find just one of the content types. Just skip over it and check the next one.
- Do not delete your module from the system table on uninstall.
- For removing node types, look at using node_type_delete (or at least the code from there since as one of the comments notes, it doesn't check if the content type still still exists) instead since fields may have been reused in other content types by people and uninstalling your module will totally break those.
- Your hook_access() and hook_perm() are redundant. Your module isn't defining its own node type, it's importing them in to the node system built in by Drupal so now there are two sets of permissions and your modules hook_access never gets called. See comments here: http://api.drupal.org/api/drupal/developer%21hooks%21node.php/function/h...

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

If you reopen this please keep in mind that we are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Review of other projects added