[D7] User Schedule

Branch

Projects should be moved into proper branches, not as part of the master branch (see Move from a master to a major version branch).

PAReview

Quite a few coding style errors. Use PAReview.sh or use ventral.org's online checking.
http://ventral.org/pareview/httpgitdrupalorgsandboxtakim1451878git

Manual Review

- Your .info file should contain a description and at least a core version (looking over your code I see D6 hooks), as well as any possible module dependencies (Date?).
- You implement hooks that do absolutely nothing (user_schedule_user, user_schedule_validate)
- You define MOD_IMAGE_PATH but never use it, nor does the directory exist.

- To view all users schedule in same graph : path is schedule

- You never define the 'schedule' path, only user_schedule
- Your function theme_user_schedule_graph has the $uid parameter, and has logic for if the uid is not null, yet it is never used, it will always be null (you're calling it from your menu hook only)

Hi takim,
just few notes about your module

1) Is your module for Drupal 6 or for Drupal 7? Looking at the code there is a hook_nodeapi, so it should be for Drupal 6, but it would be great if you add this info on your review request.

2) you should switch from master git branch to 6.x-1.x branch

3) your module's hook_nodeapi and hook_user seem empty, are they there for future developements?

4) you should at least add on your .info file the following lines:

dependencies[] = date_api
core = 6.x

PAReview.sh

Still quite a few coding style issues found from PAReview.sh. ventral.org review of your module

Manual

This is the doxygen comment for your user_schedule_form function.

/**
 *
 * @param <type> $node
 * @param <type> $form_state
 * @return <type>
 */

You should provide a one line comment about what it does (we know it's a form, but you should specify what form it creates). Also, the < type > portion should be an actual type. You also want a quick description of each @param and the @return. This is only in cases where you explicitly commented @param and @return, and in many cases (such as hooks), you don't necessarily need to type out the @param/@return or describe them as they have been already done in the initial hook.

So in this case, you would see something like:

/**
 * Some comment about this form here.
 *
 * @param stdClass $node A node.
 * @param array $form_state Current state of form.
 * @return array The form array.
 */

Or better yet (since you don't have any unique parameters exclusive to your module for this form):

/**
 * Some comment about this form here.
 */

You also don't need to specify the version of your module in your .info file, that is done automatically when the Drupal packaging script creates your module's tar.

Hi, sorry for the delay! As there are currently hundreds of applications waiting for a review but only a handful active reviewer, everything takes a while. We do really need more hands in the application queue and highly recommend to get a review bonus so we can come back to your application sooner

Manual review:

• If you implement a hook, your comment should also say that - like for user_schedule_form, your block comment should start with Implements hook_form - like you do at user_schedule_menu().
• At $form['title'] in user_schedule_form you do a check_plain on $type->title_label, why, is not an input value.
• It seems you are inserting information in the database without filtering the form values (security).
• You are adding a non existing js: drupal_add_js(drupal_get_path('module', 'user_schedule') . '/js/highcharts.js', 'module');
• In theme_user_graph you construct your dbquery: $results = db_query($sql, 1, 'user_schedule'); Why do you add 1 here as an argument?
• In the same function - you save to db_query in $result, after that you you add javascript, and after a while you come back to the $result, which makes the function hard to follow, and you should really check if your output has something before doing more.
• You do not need to have $output empty for the beginning, but you should check if it has a value before outputting it.
• It seems that you are printing out stuff, even if the result is none (in theme).
• Also, you have some new errors for your commeting style: http://ventral.org/pareview/httpgitdrupalorgsandboxtakim1451878git

Seems that you have some more work to do, but please ask if you do not know what it is :-) We are here to help each other.

Idle Application

This thread has been idle, in the 'needs work' state with no activity for several months. Therefore, I am assuming that you are no longer pursuing this application, and marking it as closed (won't fix).

If this is incorrect, and you are still pursuing this application, then please feel free to re-open the thread and set the issue status to "needs work" or "needs review", depending on the current status of your code.

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://drupal.org/node/1454042

Project 2: https://drupal.org/node/2084877

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

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

I'm evaluating the 7.x branch since that seems to be the most recent (only 1 branch is required for application).

Master Branch

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

• A lot of doc and style issues are being reported here: http://pareview.sh/pareview/httpgitdrupalorgsandboxtakim1451878git.
• In D7 user_schedule_perm should be renamed user_schedule_permission.
• There's a lot of custom time/date handling being done in user_schedule_graph() - is it possible to use strtotime() to simplify this?
• Can you use a hook_requirements() to make sure that the js library was successfully downloaded and installed?
• Can you move drupal_add_css(drupal_get_path('module', 'user_schedule') . '/css/user_schedule.css', 'module'); to where it's being used (I think on the graph page?) instead of every drupal page load?

It's not often we seeD7 modules providing their own content types anymore, have you thought about making this a Field instead? I'm not quite sure what the use case is so that may not be appropriate. But as a Field you could add this functionality to any content type.

----
Top Shelf Modules - Crafted, Curated, Contributed.

added a review comment

corrected project page url

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

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