[D7] Context Cookie

First of all, you need to update your application page with more information as the application instruction, especially on step 5.

Second, please review and fix all errors was reported at http://pareview.sh/pareview/httpgitdrupalorgsandboxjdaglees1850482

FILE: ...ww/drupal-7-pareview/pareview_temp/plugins/context_condition_cookie.inc
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 2 WARNING(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
6 | WARNING | @author tags are not usually used in Drupal, because over time
| | multiple contributors will touch the code anyway
9 | WARNING | Class name must be prefixed with the project name
| | "ContextCookie"
--------------------------------------------------------------------------------

FILE: ...www/drupal-7-pareview/pareview_temp/plugins/context_cookie_reaction.inc
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
12 | WARNING | Unused variable $values.
--------------------------------------------------------------------------------

FILE: /var/www/drupal-7-pareview/pareview_temp/context_cookie.info
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
5 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

FILE: /var/www/drupal-7-pareview/pareview_temp/context_cookie.module
--------------------------------------------------------------------------------
FOUND 19 ERROR(S) AFFECTING 19 LINE(S)
--------------------------------------------------------------------------------
3 | ERROR | The second line in the file doc comment must be " * @file"
4 | ERROR | The third line in the file doc comment must contain a description
| | and must not be indented
30 | ERROR | Whitespace found at end of line
31 | ERROR | Line indented incorrectly; expected 2 spaces, found 3
32 | ERROR | Array indentation error, expected 5 spaces but found 4
37 | ERROR | Array indentation error, expected 5 spaces but found 4
38 | ERROR | Array closing indentation error, expected 3 spaces but found 2
47 | ERROR | Array indentation error, expected 4 spaces but found 6
48 | ERROR | Array indentation error, expected 8 spaces but found 10
49 | ERROR | Array indentation error, expected 12 spaces but found 14
50 | ERROR | Array indentation error, expected 12 spaces but found 14
51 | ERROR | Array indentation error, expected 8 spaces but found 10
52 | ERROR | Array indentation error, expected 4 spaces but found 6
53 | ERROR | Array indentation error, expected 4 spaces but found 6
54 | ERROR | Array indentation error, expected 8 spaces but found 10
55 | ERROR | Array indentation error, expected 12 spaces but found 14
56 | ERROR | Array indentation error, expected 12 spaces but found 14
57 | ERROR | Array indentation error, expected 8 spaces but found 10
58 | ERROR | Array indentation error, expected 4 spaces but found 6
--------------------------------------------------------------------------------

FILE: ...ww/drupal-7-pareview/pareview_temp/plugins/context_condition_cookie.inc
--------------------------------------------------------------------------------
FOUND 13 ERROR(S) AFFECTING 12 LINE(S)
--------------------------------------------------------------------------------
9 | ERROR | Class name must begin with a capital letter
9 | ERROR | Class name must use UpperCamel naming without underscores
14 | ERROR | Public method name "context_condition_cookie::condition_values"
| | is not in lowerCamel format, it must not contain underscores
21 | ERROR | Public method name "context_condition_cookie::condition_form" is
| | not in lowerCamel format, it must not contain underscores
50 | ERROR | Whitespace found at end of line
51 | ERROR | Line indented incorrectly; expected 2 spaces, found 4
52 | ERROR | Expected 5 space(s) before asterisk; 3 found
53 | ERROR | Expected 5 space(s) before asterisk; 3 found
54 | ERROR | Public method name
| | "context_condition_cookie::condition_form_submit" is not in
| | lowerCamel format, it must not contain underscores
66 | ERROR | No scope modifier specified for function "execute"
69 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
72 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
78 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
--------------------------------------------------------------------------------

FILE: ...www/drupal-7-pareview/pareview_temp/plugins/context_cookie_reaction.inc
--------------------------------------------------------------------------------
FOUND 4 ERROR(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
7 | ERROR | Class name must begin with a capital letter
7 | ERROR | Class name must use UpperCamel naming without underscores
11 | ERROR | Public method name "context_cookie_reaction::options_form" is not
| | in lowerCamel format, it must not contain underscores
35 | ERROR | Whitespace found at end of line
--------------------------------------------------------------------------------

After you resolve those errors, I will review your source code. Please let me know if you need any help to fix them.

Once you resolve the above coder standards, I think things are fairly ready to go. The module works as advertised. I use it on a few production sites. The long problem (not sure it is git vetted blocker, but is a release blocker) is https://drupal.org/node/2225525

Daglees, there's still a lot of whitespace issues according to PAReview. And some low hanging fruit. Removing the low hanging fruit will avoid a lot of unecessary discussion here in the forum. So just do it.

If don't want to do that, at the very least these things must be address:

README.txt missing
Name the class context_cookie_condition
Master branch: http://drupal.org/node/1127732

Non-blocking issues:

• FILE: context_cookie/context_cookie.module
  

  $form['context_cookie'] = array(
      '#type' => 'textarea',
      '#title' => t('Keys'),
      '#description' => t('List of cookies separated by newline that are relevant and should be avialable in context. Variables are made available in the "cookie" namespace with the attribute being the key. If the cookie value is an array the attribute names will be generated as follows: key.subkey, key.subkey2, key.subkey2.subsubkey.'),
      '#default_value' => implode("\n", variable_get('context_cookie', array())),
    );

  Miss spelled avialable.

• FILE: context_cookie/context_cookie.module
  The third line in the file doc comment must contain a description and must not be indented

This has been RTBCed now for over week. I'll give it another week to see if any of the other git administrators have feedback, then I'll promote it to a full project.

One additional thing I noticed today (again, not a blocker) is

/**
 * Convert the keys text to an array.
 */
function context_cookie_settings_form_validate($form, &$form_state) {
  $keys = &$form_state['values']['context_cookie'];
  $keys = explode("\r\n", trim($keys));
  $keys = drupal_map_assoc($keys);
}

There is no need to pass by reference.

@Daglees, thankyou for your contribution.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. Yes http://pareview.sh/pareview/httpgitdrupalorgsandboxjdaglees1850482git reported some warnings, I know all warnings can't be fixed in your case but have a look again and do fix some new warnings.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes. If "no", list security issues identified.

Coding style & Drupal API usage

1. (*) I used this module and access the admin/structure/context/settings/cookie page. It produces following warnings when I submitted this form.
   
   

   And I checked another pages and found given warning is coming on every page:
   
   

   It seems blocker from my side, need to fix it.

2. I have one suggestion, it would be good if you add configure = admin/structure/context/settings/cookie in .info file, to enable it directly access from module page.
3. (+) I am little bit unsure about following code what exactly $keys is doing here, please add your comment on this.
   

   function context_cookie_settings_form_validate($form, &$form_state) {
     $keys = $form_state['values']['context_cookie'];
     $keys = explode("\r\n", trim($keys));
     $keys = drupal_map_assoc($keys);
   }
   

4. Personally, I think it would be useful if you create separate permission instead using default administer site configuration.
5. hook_help() is missing in your module.
6. Please add your comment about following code what $name hold here?
   

   foreach ($contexts as $context) {
         if (is_array($context->reactions[$this->plugin])) {
           if (extract($context->reactions[$this->plugin]) == 7) {
             $_COOKIE[$name] = $value;
             $expire = strtotime('+' . $expire);
             setrawcookie($name, rawurlencode($value), $expire, $path, $domain, $secure, $httponly);
           }
         }
       }
   

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

As I am not a git administrator, so I would recommend you, please help to review other project applications to get a review bonus. This will put you on the high priority list, then git administrators will take a look at your project right away :-)

Thanks Again!

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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

I really really need the functionality it provides. I tried using the experiment and indeed i get the same errors shown in the pictures on comment 16. Any updates would be greatly appreciated. Is this project abandoned?