Autofill

Priority update as per http://drupal.org/node/894256

Hi rcb.ferreira,
It appears that your git repo for the 6.x-1.x branch is missing js/autofill.js, so the module is not working properly. Please add that file to the repository, and set this back to Needs Review. Thanks!

The file http://drupalcode.org/sandbox/rui/1176850.git/blob/refs/heads/6.x-1.x:/j... looks like it is an external library?

From the Drupal.org Git usage policy:

DO NOT include code from a non-Drupal project in the repository. If your module requires non-Drupal code, such as a third-party JavaScript/PHP library, provide a link to where the other code can be downloaded and instructions on how to install it.

Hi Rui,

1. Lines in README.txt should not exceed 80 characters.
And rename README to README.txt.

2. "if (empty($field['id']))" always use "{}" around if/else statements, even if it is just one line.
This often leads to bugs if you add lines later. The same rule applies for "for (), while (), foreach ()"

3. For some reason I don't see this module on your profile page.

Thanks,

Minnur

Review of the 6.x-1.x branch:

• README.txt is missing, see the guidelines for in-project documentation.
• Remove "version" from the info file, it will be added by drupal.org packaging automatically.
• All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
  

  ./js/sample.js ./autofill.info ./autofill.module ./autofill.install ./autofill.admin.inc
  

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.

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

See http://ventral.org/pareview/httpgitdrupalorgsandboxrui1176850git

autofill.module:
+46: [minor] Missing parenthesis after function name

* Implements hook_perm.
->
* Implements hook_perm().

autofill.admin.inc
86 | ERROR | Constants must be uppercase; expected SPECIFIC but found specific
86 | ERROR | Constants must be uppercase; expected GENERAL but found general

Be aware of using unescaped " in a string with "" ;-)
form_error($form, t("%field-id doesn\'t contain a CSS class or ID identifier (e.g. <em>#specific</em> for <em>id="specific"</em> and <em>.general</em> for <em>class="general"</em>).", array('%field-id' => $fields[$delta]['id'])));
->

form_error($form, t('%field-id doesn\'t contain a CSS class or ID identifier (e.g. <em>#specific</em> for <em>id="specific"</em> and <em>.general</em> for <em>class="general"</em>).', array('%field-id' => $fields[$delta]['id'])));

js/sample.js
21 | ERROR | Files must end in a single new line character
add an empty line to the end of this file

Switched back to needs review, so in-depth reviews won't be blocked by coding standart issues.

// double post, sorry, internet was hanging.

Did a fairly quick review, but everything looks good!

Review of the 6.x-1.x branch:

• Drupal Code Sniffer has found some code style issues (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. Go and review some other project applications, so we can get back to yours sooner.

manual review:

• sample.js: always use 2 spaces per level for indentation, not one.
• autofill_js(): Function doc block is wrong.
• autofill_menu(): why do you need drupal_get_path() there? "/autofill" as path prefix should also do it.
• autofill_js(): you should use drupal_json() here.
• The page callbck to autofill_js() is unprotected. This means that anyone can retrieve a list of default form values used on your site. So you must never use sensitive information in your default values, as this can be leaked to anyone. This must be clearly stated at the project page. Or you could make it more secure: the ajax call passes the a path argument to Drupal and your page callback checks if the user even has access to that path. Of course this would mean that you only return default values for that particular path and not just everything. You would have to store path information with your default values.
• autofill_form(): this is not a hook, this is a form constructor. See http://drupal.org/node/1354#forms
• autofill_settings_form_validate(): indentation errors, always use 2 spaces per level, not 3.

This application has been marked as 'needs work' for 19 weeks, and there is no contact form on the users profile page. I am closing this on.

@rcb.ferreira, if this is wrong, please reopen.

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