[D7] Views Superglobals Default

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

• Coder 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.

FILE: ...7-pareview/pareview_temp/views_plugin_argument_default_superglobals.inc
--------------------------------------------------------------------------------
FOUND 23 ERROR(S) AND 1 WARNING(S) AFFECTING 13 LINE(S)
--------------------------------------------------------------------------------
 13 | ERROR   | Class name must begin with a capital letter
 13 | ERROR   | Class name must use UpperCamel naming without underscores
 14 | ERROR   | Missing function doc comment
 14 | ERROR   | Method name
    |         | "views_plugin_argument_default_superglobals::option_definition"
    |         | is not in lowerCamel format, it must not contain underscores
 14 | ERROR   | No scope modifier specified for function "option_definition"
 22 | ERROR   | Missing function doc comment
 22 | ERROR   | Method name
    |         | "views_plugin_argument_default_superglobals::options_form" is
    |         | not in lowerCamel format, it must not contain underscores
 22 | ERROR   | No scope modifier specified for function "options_form"
 25 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
    |         | question marks
 47 | WARNING | Avoid backslash escaping in translatable strings when possible,
    |         | use "" quotes instead
 52 | ERROR   | Missing function doc comment
 52 | ERROR   | Method name
    |         | "views_plugin_argument_default_superglobals::get_argument" is
    |         | not in lowerCamel format, it must not contain underscores
 52 | ERROR   | No scope modifier specified for function "get_argument"
 89 | ERROR   | Functions must not contain multiple empty lines in a row; found
    |         | 2 empty lines
 94 | ERROR   | Line indented incorrectly; expected 6 spaces, found 8
 95 | ERROR   | Line indented incorrectly; expected 8 spaces, found 12
 96 | ERROR   | else must start on a new line
 96 | ERROR   | Use "elseif" in place of "else if"
 96 | ERROR   | Line indented incorrectly; expected 6 spaces, found 8
 97 | ERROR   | Line indented incorrectly; expected 8 spaces, found 12
 98 | ERROR   | Line indented incorrectly; expected 6 spaces, found 8
 98 | ERROR   | Line indented incorrectly; expected 6 spaces, found 8
 98 | ERROR   | else must start on a new line
 99 | ERROR   | Line indented incorrectly; expected 8 spaces, found 12
--------------------------------------------------------------------------------


FILE: ...r/www/drupal-7-pareview/pareview_temp/views_superglobals_default.module
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 2 WARNING(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 17 | WARNING | A comma should follow the last multiline array item. Found: )
 18 | WARNING | A comma should follow the last multiline array item. Found: )
--------------------------------------------------------------------------------

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

ryan.armstrong,

Thank you for your interesting module. With the exception of the handler's method names, all other errors can be fixed, so it is my advise you do so.

The example in your image is incorrect - it says "if you wanted to use $_GET ['variable']['subvariable'], you would enter example" which doesn't look like it would work. That's in your help text in views_plugin_argument_default_superglobals::options_form().

Especially in the cases of _GET and _POST, you want to be running that through some sanitization, check_plain or filter_xss.

Setting to needs work for the security issue, otherwise this would be RTBC from me.

----
Top Shelf Modules - Enterprise modules from the community for the community.

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

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

Updating description. I removed the error message if the variable wasn't found as there may be cases where that isn't an error.

All feedback has been incorporated into the project. Marking as RTBC.

Manual Review

drupal_map_assoc might be useful for these large (duplicate) arrays.

    $superglobal_options = array(
      '$_SERVER' => '$_SERVER',
      '$_GET' => '$_GET',
      '$_POST' => '$_POST',
      '$_FILES' => '$_FILES',
      '$_COOKIE' => '$_COOKIE',
      '$_SESSION' => '$_SESSION',
      '$_REQUEST' => '$_REQUEST',
      '$_ENV' => '$_ENV',
    );

However, that isn't a blocker...

Thanks for your contribution, ryan.armstrong!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.