[D6] Storm Organization Holiday Manager

Hi D34dMan,

Please fix the open bugs first, then move this back to "needs review."

Let's go ahead and move this to needs review so it doesn't get lost. Someone can review those particular issue fixes as part of reviewing the larger project.

Why can't you include your code with the Storm contrib project? Have you opened an issue in the Storm contrib issue queue to ask for feedback on this? Or did you contact the maintainers in any other way?

There are still files other than README.txt in the master branch, make sure to remove them. See also step 5 in http://drupal.org/node/1127732
Review of the 6.x-1.0-alpha1 branch:

• Run coder to check your style, some issues were found (please check the Drupal coding standards):
  

  Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards
  
  sites/all/modules/pareview_temp/test_candidate/storm_ohm.module:
   +2: [minor] Commits to the Git repository do not require the CVS $Id$ keyword in each file.
   +83: [normal] use quotes around a string literal array index, this is not only a style issue, but a known performance problem
   +84: [normal] use quotes around a string literal array index, this is not only a style issue, but a known performance problem
   +201: [normal] The $string argument to t() should not begin or end with a space.
  
  sites/all/modules/pareview_temp/test_candidate/storm_ohm.install:
   +2: [minor] Commits to the Git repository do not require the CVS $Id$ keyword in each file.
  
  Status Messages:
   Coder found 1 projects, 4 files, 3 normal warnings, 2 minor warnings, 0 warnings were flagged to be ignored
  

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.
• README.txt is missing, see the guidelines for in-project documentation.
• Remove "project" from the info file, it will be added by drupal.org packaging automatically.
• Remove all old CVS $Id tags, they are not needed anymore.
  

  storm_ohm.info:1:;$Id$
  storm_ohm_info.tpl.php:2:// $Id$
  storm_ohm.install:2:// $Id$
  storm_ohm.module:2:// $Id$
  

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

manual review:

• wrong branch name, 6.x-1.0-alpha1 should be 6.x-1.x

@D34dMan has been contacted to ask if the application is abandoned.

After ten weeks with a status of needs work: the applicant may be contacted by a reviewer to determine whether the application was indeed abandoned. The action taken by the reviewer should be documented in the project application issue.

http://drupal.org/node/894256

If there are nothing hat you need in 6.x-1.0-alpha, you should delete it, if there is something you need later on - rename the branch to something like alpha, so the users does not get confused in which branch they should use.

Changing the git repository clone command to reflect the latest development snapshot.

> Created tag 6.x-1.0-alpha1
Small hint: You should not create tags, unless you plan to release it.
As you are not vetted user, you cannot do releases, so tags are unnecessary and should be removed.

Branch 6.x-1.x is correct.

The response time for a review is now approaching 4 weeks.
Get a review bonus and we will come back to your application sooner.
See: http://drupal.org/node/1410826

(1)
> Line exceeds 80 characters
Doc-Comments should be limited to 80 characters for easing printout.

(2)
All files, functions should have a doc comment, in the following format
/**
* What the file is about ....
*/

(3)
Many other issues where detected by http://ventral.org/pareview/httpgitdrupalorgsandboxd34dman1309636git

You should repeat the automated review before going to "needs review".

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
Review of the 6.x-1.x branch:

• Remove "project" from the info file, it will be added by drupal.org packaging automatically.
• Drupal Code 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.

manual review:

1. There is a 6.x-1.0-alpha1 git branch, please remove it.
2. "'access arguments' => array('Administer Storm OHM'),": permissions should be all lower case.
3. the permission is missing.
4. storm_ohm_storm_holidays_alter(): why the line break in the middle of the function definition? Also elsewhere. Code lines are allowed to exceed 80 characters.
5. You should remove all variables that your modules defines in hook_uninstall().
6. storm_ohm_admin_manage_form_submit(): do not use check_plain() on literal string variables, you should only sanitize user provided input. Please read http://drupal.org/node/28984 again.
7. storm_ohm_admin_manage_form_submit(): do not sanitize stuff when you are writing to the database. "When handling data, the golden rule is to store exactly what the user typed. When a user edits a post they created earlier, the form should contain the same things as it did when they first submitted it. This means that conversions are performed when content is output, not when saved to the database" from http://drupal.org/node/28984

i suppose it would be better leaving the status at "needs work".

I find "postponed" confusing...

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 :-)

Multiple Applications

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

Project 1: #1962038: [D7] PM Vista
Project 2: #1309746: [D6] Storm Organization Holiday Manager

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.

switching to branch 6.x-1.x from 6.x-1.0-alpha1