Short description:

The ePaper Plugin helps you to embed Pageflips published by 1000grad
ePaper. Integrate your digital magazines directly onto your website -
represented in form of a simple link, image or an animated picture.

The plugin is a great help for 1000grad ePaper-users using the drupal-cms. You can
purchase additional ePapers at the following website href="http://www.1000grad-epaper.de">www.1000grad-epaper.de. All
ePapers are available as Flash and HTML5. The reader will be swap
automatically when the ePaper is used on a mobile device.

Features:
* The ePaper Admin manages all your publications
* easily integrate ePapers into your site as Flash teaser, pop-up, via Lightbox or as a simple link
* combine several ePaper to a group
* easy of use administration interface

Comments

sreynen’s picture

Status: Needs review » Needs work

Project link: http://drupal.org/sandbox/1000.grad.digital/1302394

1000.grad.digital, at a quick skim, this seems to have a lot of problems that could be caught by the Coder module. Please run that on the "minor" setting, fix any problems it identifies, and change this status to "needs review."

1000.grad.digital’s picture

Status: Needs work » Needs review
klausi’s picture

Status: Needs review » Needs work

Review of the 7.x-1.x 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/epaper_admin.module:
     +158: [minor] There should be no trailing spaces
    
    sites/all/modules/pareview_temp/test_candidate/epaper_admin_node/epaper_admin_node.install:
     +22: [minor] There should be no trailing spaces
     +27: [minor] There should be no trailing spaces
    
    sites/all/modules/pareview_temp/test_candidate/epaper_admin.ajax.php:
     +-1: [minor] @file block missing
     +16: [minor] do not use mixed case (camelCase), use lower case and _
     +17: [minor] do not use mixed case (camelCase), use lower case and _
     +18: [minor] Control statements should have one space between the control keyword and opening parenthesis
     +18: [minor] do not use mixed case (camelCase), use lower case and _
     +19: [minor] do not use mixed case (camelCase), use lower case and _
     +19: [minor] Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
     +20: [minor] else statements should begin on a new line
     +21: [minor] do not use mixed case (camelCase), use lower case and _
     +21: [minor] Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
     +23: [minor] the final ?> should be omitted from all code files
     +24: [minor] There should be no trailing spaces
    
    sites/all/modules/pareview_temp/test_candidate/epaper_admin.class.inc:
     +71: [minor] There should be no trailing spaces
     +409: [minor] in most cases, replace the string function with the drupal_ equivalent string functions
     +424: [minor] There should be no trailing spaces
     +526: [minor] There should be no trailing spaces
    
    Status Messages:
     Coder found 1 projects, 8 files, 19 minor warnings, 0 warnings were flagged to be ignored
    
  • Lines in README.txt should not exceed 80 characters, see the guidelines for in-project documentation.
  • Remove LICENSE.txt, it will be added by drupal.org packaging automatically.
  • Remove the translations folder, translations are done on http://localize.drupal.org
  • Remove "version" from the info file, it will be added by drupal.org packaging automatically.
  • Remove "project" from the info file, it will be added by drupal.org packaging automatically.
  • Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically.
  • @file doc block is missing in the module file, see http://drupal.org/node/1354#files .
  • The "?>" PHP delimiter at the end of files is discouraged, see http://drupal.org/node/318#phptags
    ./epaper_admin.ajax.php
    
  • epaper_admin.module in epaper_admin.info: It's only necessary to declare files[] if they declare a class or interface.
  • epaper_admin.install in epaper_admin.info: It's only necessary to declare files[] if they declare a class or interface.
  • ./epaper_admin_node/epaper_admin_node.install: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
    function epaper_admim_node_install() {
    function epaper_admim_node_uninstall() {
    
  • Bad line endings were found, always use unix style terminators. See http://drupal.org/coding-standards#indenting
    ./LIESMICH.txt:                                           UTF-8 Unicode English text, with CRLF line terminators
    ./README.txt:                                             UTF-8 Unicode English text, with CR line terminators
    ./epaper_admin_node/epaper_admin_node.info:               ASCII text, with CRLF line terminators
    ./epaper_admin.info:                                      ASCII text, with CRLF line terminators
    ./LICENSE.txt:                                            ASCII English text, with CRLF line terminators
    
  • Assignments should have a space before and after the operator, see http://drupal.org/node/318#operators
    ./epaper_admin.system.inc:23:function epaper_admin_system($option='list', $type='epaper', $id=0) {
    ./epaper_admin.system.inc:295:function epaper_admin_edit($list, $id=0) {
    ./epaper_admin.system.inc:304:function epaper_admin_form_add_edit($form, $form_state, $id=0) {
    ./epaper_admin.system.inc:528:function epaper_admin_delete($id=0) {
    ./epaper_admin.system.inc:604:function epaper_admin_form_source($form, $form_state, $id=0) {
    ./epaper_admin.system.inc:831:function epaper_admin_group_edit($list, $id=0) {
    ./epaper_admin.system.inc:840:function epaper_admin_form_group_add_edit($form, $form_state, $id=0) {
    ./epaper_admin.system.inc:996:function epaper_admin_group_delete($id=0) {
    ./epaper_admin.system.inc:1072:function epaper_admin_form_group_source($form, $form_state, $id=0) {
    ./epaper_admin.class.inc:49:  public function getAllEpaper($sql_where='') {
    ./epaper_admin.class.inc:178:  public function deleteEpaper($where=array()) {
    ./epaper_admin.class.inc:214:  public function getStatusName($int, $blank=FALSE) {
    ./epaper_admin.class.inc:286:  public function getEpaperSource($data, $small=FALSE) {
    ./epaper_admin.class.inc:354:  public function getPreviewImage($path, $id='') {
    ./epaper_admin.class.inc:379:  public function getPreviewImageType($id='') {
    ./epaper_admin.class.inc:403:  public function getOnclickType($id='') {
    ./epaper_admin.class.inc:614:  public function deleteGroup($where=array()) {
    
  • Remove all old CVS $Id tags, they are not needed anymore.
    LIESMICH.txt:1:$Id: LIESMICH.txt, 2011/08/16 11:40:13 Exp $
    
    README.txt:1:$Id: README.txt, 2011/08/16 08:56:13 Exp $
    
  • Classes and Interfaces should use UpperCamel naming. See http://drupal.org/node/608152
    ./epaper_admin.class.inc:491:class epaper_group extends epaper {
    

This automated report was generated with PAReview.sh, your friendly project application review script. Please report any bugs to klausi.

manual review:
swfobject.js: appears to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms. The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.

1000.grad.digital’s picture

Status: Needs work » Needs review
patrickd’s picture

patrickd’s picture

Status: Needs work » Needs review

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

1000.grad.digital’s picture

needs another review

frob’s picture

Still quite a few coding standards related errors. Also I noticed the user name is branded with the service. Is user 1000.grad.digital a user that is intended for a single individual? Only individuals can have the ability to promote sandbox projects into full projects.

As part of my manual code review, I am trying to understand what your project is doing. There are two modules: epaper_admin and epaper_admin_node. What is the reason for the two modules; epaper_admin_node only has a hook_form_alter. Will either module work without the other? Aside from that, there is a fileter and also a hook_node_view and they both seem to be doing the same thing.
In hook_node_view

        // Source - Replace.
        $epapergroup->replaceEpaperSource($node->content[$val][0]['#markup']);
        $epapergroup->replaceEpaperGroupSource($node->content[$val][0]['#markup']);

and in hook_filter_process

  // Source - Replace.
  $epapergroup->replaceEpaperSource($text);
  $epapergroup->replaceEpaperGroupSource($text);

Since this is happening in the hook_node_view and you are checking to make sure that the view_mode is full, this is happening everytime a node is fully viewed. Meaning if it is loading on every full node view why have a filter on it too? Your approach is confusing me. Please elaborate -- the automated code review is following.

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 7.x-1.x branch:

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. Get a review bonus and we will come back to your application sooner.


FILE: ...-7-pareview/sites/all/modules/pareview_temp/test_candidate/LIESMICH.txt
--------------------------------------------------------------------------------
FOUND 30 ERROR(S) AND 2 WARNING(S) AFFECTING 24 LINE(S)
--------------------------------------------------------------------------------
  29 | WARNING | Line exceeds 80 characters; contains 82 characters
 125 | WARNING | Line exceeds 80 characters; contains 81 characters
 171 | ERROR   | Variable "epaperNr" is camel caps format. do not use mixed
     |         | case (camelCase), use lower case and _
 176 | ERROR   | Variable "epaperData" is camel caps format. do not use mixed
     |         | case (camelCase), use lower case and _
 176 | ERROR   | Constants must be uppercase; expected EPAPERNR but found
     |         | epaperNr
 177 | ERROR   | Variable "epaperData" is camel caps format. do not use mixed
     |         | case (camelCase), use lower case and _
 178 | ERROR   | No space before comment text; expected "//
     |         | var_dump($epaperData);" but found "//var_dump($epaperData);"
 189 | ERROR   | Expected "foreach (...) {\n"; found "foreach(...) {\n"
 189 | ERROR   | Line indented incorrectly; expected 0 spaces, found 16
 190 | ERROR   | Variable "epaperData" is camel caps format. do not use mixed
     |         | case (camelCase), use lower case and _
 192 | ERROR   | Variable "epaperData" is camel caps format. do not use mixed
     |         | case (camelCase), use lower case and _
 194 | ERROR   | No space before comment text; expected "//
     |         | var_dump($epaperData);" but found "//var_dump($epaperData);"
 206 | ERROR   | Variable "groupNr" is camel caps format. do not use mixed case
     |         | (camelCase), use lower case and _
 212 | ERROR   | Variable "groupData" is camel caps format. do not use mixed
     |         | case (camelCase), use lower case and _
 212 | ERROR   | Constants must be uppercase; expected GROUPNR but found
     |         | groupNr
 212 | ERROR   | Whitespace found at end of line
 213 | ERROR   | No space before comment text; expected "//
     |         | var_dump($groupData);" but found "//var_dump($groupData);"
 213 | ERROR   | There must be no blank line following an inline comment
 215 | ERROR   | Expected "foreach (...) {\n"; found "foreach(...) {\n"
 215 | ERROR   | Line indented incorrectly; expected 0 spaces, found 16
 216 | ERROR   | Variable "epaperData" is camel caps format. do not use mixed
     |         | case (camelCase), use lower case and _
 217 | ERROR   | Variable "epaperData" is camel caps format. do not use mixed
     |         | case (camelCase), use lower case and _
 218 | ERROR   | No space before comment text; expected "//
     |         | var_dump($epaperData);" but found "//var_dump($epaperData);"
 231 | ERROR   | Expected "foreach (...) {\n"; found "foreach(...) {\n"
 231 | ERROR   | Line indented incorrectly; expected 0 spaces, found 16
 233 | ERROR   | Expected "foreach (...) {\n"; found "foreach(...) {\n"
 233 | ERROR   | Line indented incorrectly; expected 2 spaces, found 20
 234 | ERROR   | Variable "epaperData" is camel caps format. do not use mixed
     |         | case (camelCase), use lower case and _
 235 | ERROR   | Variable "epaperData" is camel caps format. do not use mixed
     |         | case (camelCase), use lower case and _
 236 | ERROR   | No space before comment text; expected "//
     |         | var_dump($epaperData);" but found "//var_dump($epaperData);"
 242 | ERROR   | Whitespace found at end of line
 252 | ERROR   | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: ...al-7-pareview/sites/all/modules/pareview_temp/test_candidate/README.txt
--------------------------------------------------------------------------------
FOUND 30 ERROR(S) AFFECTING 22 LINE(S)
--------------------------------------------------------------------------------
 166 | ERROR | Variable "epaperNr" is camel caps format. do not use mixed case
     |       | (camelCase), use lower case and _
 171 | ERROR | Variable "epaperData" is camel caps format. do not use mixed
     |       | case (camelCase), use lower case and _
 171 | ERROR | Constants must be uppercase; expected EPAPERNR but found
     |       | epaperNr
 172 | ERROR | Variable "epaperData" is camel caps format. do not use mixed
     |       | case (camelCase), use lower case and _
 173 | ERROR | No space before comment text; expected "//
     |       | var_dump($epaperData);" but found "//var_dump($epaperData);"
 184 | ERROR | Expected "foreach (...) {\n"; found "foreach(...) {\n"
 184 | ERROR | Line indented incorrectly; expected 0 spaces, found 16
 185 | ERROR | Variable "epaperData" is camel caps format. do not use mixed
     |       | case (camelCase), use lower case and _
 187 | ERROR | Variable "epaperData" is camel caps format. do not use mixed
     |       | case (camelCase), use lower case and _
 189 | ERROR | No space before comment text; expected "//
     |       | var_dump($epaperData);" but found "//var_dump($epaperData);"
 201 | ERROR | Variable "groupNr" is camel caps format. do not use mixed case
     |       | (camelCase), use lower case and _
 207 | ERROR | Variable "groupData" is camel caps format. do not use mixed case
     |       | (camelCase), use lower case and _
 207 | ERROR | Constants must be uppercase; expected GROUPNR but found groupNr
 207 | ERROR | Whitespace found at end of line
 208 | ERROR | No space before comment text; expected "//
     |       | var_dump($groupData);" but found "//var_dump($groupData);"
 208 | ERROR | There must be no blank line following an inline comment
 210 | ERROR | Expected "foreach (...) {\n"; found "foreach(...) {\n"
 210 | ERROR | Line indented incorrectly; expected 0 spaces, found 16
 211 | ERROR | Variable "epaperData" is camel caps format. do not use mixed
     |       | case (camelCase), use lower case and _
 212 | ERROR | Variable "epaperData" is camel caps format. do not use mixed
     |       | case (camelCase), use lower case and _
 213 | ERROR | No space before comment text; expected "//
     |       | var_dump($epaperData);" but found "//var_dump($epaperData);"
 226 | ERROR | Expected "foreach (...) {\n"; found "foreach(...) {\n"
 226 | ERROR | Line indented incorrectly; expected 0 spaces, found 16
 228 | ERROR | Expected "foreach (...) {\n"; found "foreach(...) {\n"
 228 | ERROR | Line indented incorrectly; expected 2 spaces, found 20
 229 | ERROR | Variable "epaperData" is camel caps format. do not use mixed
     |       | case (camelCase), use lower case and _
 230 | ERROR | Variable "epaperData" is camel caps format. do not use mixed
     |       | case (camelCase), use lower case and _
 231 | ERROR | No space before comment text; expected "//
     |       | var_dump($epaperData);" but found "//var_dump($epaperData);"
 237 | ERROR | Whitespace found at end of line
 247 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: ...ew/sites/all/modules/pareview_temp/test_candidate/epaper_admin.ajax.php
--------------------------------------------------------------------------------
FOUND 14 ERROR(S) AFFECTING 8 LINE(S)
--------------------------------------------------------------------------------
 15 | ERROR | Last parameter comment requires a blank newline after it
 24 | ERROR | Expected "foreach (...) {\n"; found "foreach(...) {\n"
 24 | ERROR | Line indented incorrectly; expected 2 spaces, found 4
 27 | ERROR | Inline control structures are not allowed
 27 | ERROR | An operator statement must be followed by a single space
 27 | ERROR | There must be a single space before an operator statement
 28 | ERROR | Line indented incorrectly; expected 4 spaces, found 8
 28 | ERROR | Expected 0 spaces after opening bracket; 1 found
 28 | ERROR | Concat operator must be surrounded by spaces
 28 | ERROR | Expected 0 spaces before closing bracket; 1 found
 33 | ERROR | Whitespace found at end of line
 39 | ERROR | Last parameter comment requires a blank newline after it
 51 | ERROR | Space found before comma in function call
 57 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: ...w/sites/all/modules/pareview_temp/test_candidate/epaper_admin.class.inc
--------------------------------------------------------------------------------
FOUND 39 ERROR(S) AFFECTING 39 LINE(S)
--------------------------------------------------------------------------------
  14 | ERROR | The PHP 4-style "var" declaration must not be used.
  15 | ERROR | The PHP 4-style "var" declaration must not be used.
  16 | ERROR | The PHP 4-style "var" declaration must not be used.
  17 | ERROR | The PHP 4-style "var" declaration must not be used.
  18 | ERROR | The PHP 4-style "var" declaration must not be used.
  19 | ERROR | The PHP 4-style "var" declaration must not be used.
  20 | ERROR | The PHP 4-style "var" declaration must not be used.
  45 | ERROR | Last parameter comment requires a blank newline after it
 133 | ERROR | Last parameter comment requires a blank newline after it
 157 | ERROR | Whitespace found at end of line
 170 | ERROR | Last parameter comment requires a blank newline after it
 202 | ERROR | Last parameter comment requires a blank newline after it
 252 | ERROR | Last parameter comment requires a blank newline after it
 277 | ERROR | Last parameter comment requires a blank newline after it
 349 | ERROR | Last parameter comment requires a blank newline after it
 408 | ERROR | Variable "codeList" is camel caps format. do not use mixed case
     |       | (camelCase), use lower case and _
 409 | ERROR | Variable "codeList" is camel caps format. do not use mixed case
     |       | (camelCase), use lower case and _
 410 | ERROR | Variable "codeList" is camel caps format. do not use mixed case
     |       | (camelCase), use lower case and _
 411 | ERROR | Variable "codeList" is camel caps format. do not use mixed case
     |       | (camelCase), use lower case and _
 412 | ERROR | Variable "codeList" is camel caps format. do not use mixed case
     |       | (camelCase), use lower case and _
 413 | ERROR | Variable "codeList" is camel caps format. do not use mixed case
     |       | (camelCase), use lower case and _
 426 | ERROR | Last parameter comment requires a blank newline after it
 441 | ERROR | Inline control structures are not allowed
 459 | ERROR | Last parameter comment requires a blank newline after it
 491 | ERROR | Last parameter comment requires a blank newline after it
 521 | ERROR | Last parameter comment requires a blank newline after it
 573 | ERROR | Variable "typeList" is camel caps format. do not use mixed case
     |       | (camelCase), use lower case and _
 575 | ERROR | Variable "typeList" is camel caps format. do not use mixed case
     |       | (camelCase), use lower case and _
 577 | ERROR | Variable "typeList" is camel caps format. do not use mixed case
     |       | (camelCase), use lower case and _
 603 | ERROR | The PHP 4-style "var" declaration must not be used.
 604 | ERROR | The PHP 4-style "var" declaration must not be used.
 605 | ERROR | The PHP 4-style "var" declaration must not be used.
 606 | ERROR | The PHP 4-style "var" declaration must not be used.
 683 | ERROR | Last parameter comment requires a blank newline after it
 718 | ERROR | Last parameter comment requires a blank newline after it
 750 | ERROR | Last parameter comment requires a blank newline after it
 774 | ERROR | Last parameter comment requires a blank newline after it
 813 | ERROR | Last parameter comment requires a blank newline after it
 877 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: ...review/sites/all/modules/pareview_temp/test_candidate/epaper_admin.info
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 6 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: ...iew/sites/all/modules/pareview_temp/test_candidate/epaper_admin.install
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 182 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: ...view/sites/all/modules/pareview_temp/test_candidate/epaper_admin.module
--------------------------------------------------------------------------------
FOUND 5 ERROR(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
 176 | ERROR | Whitespace found at end of line
 191 | ERROR | Last parameter comment requires a blank newline after it
 205 | ERROR | Last parameter comment requires a blank newline after it
 276 | ERROR | Last parameter comment requires a blank newline after it
 293 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: .../sites/all/modules/pareview_temp/test_candidate/epaper_admin.system.inc
--------------------------------------------------------------------------------
FOUND 21 ERROR(S) AFFECTING 21 LINE(S)
--------------------------------------------------------------------------------
   19 | ERROR | Last parameter comment requires a blank newline after it
   80 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
      |       | question marks
  148 | ERROR | Whitespace found at end of line
  172 | ERROR | Whitespace found at end of line
  184 | ERROR | Last parameter comment requires a blank newline after it
  220 | ERROR | Last parameter comment requires a blank newline after it
  379 | ERROR | Last parameter comment requires a blank newline after it
  519 | ERROR | Last parameter comment requires a blank newline after it
  650 | ERROR | Last parameter comment requires a blank newline after it
  700 | ERROR | Last parameter comment requires a blank newline after it
  722 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
      |       | question marks
  737 | ERROR | Last parameter comment requires a blank newline after it
  861 | ERROR | Last parameter comment requires a blank newline after it
  864 | ERROR | Whitespace found at end of line
  871 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
      |       | question marks
 1017 | ERROR | Last parameter comment requires a blank newline after it
 1110 | ERROR | Last parameter comment requires a blank newline after it
 1220 | ERROR | Last parameter comment requires a blank newline after it
 1270 | ERROR | Last parameter comment requires a blank newline after it
 1307 | ERROR | Last parameter comment requires a blank newline after it
 1422 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: ...s/pareview_temp/test_candidate/epaper_admin_node/epaper_admin_node.info
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 5 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: ...pareview_temp/test_candidate/epaper_admin_node/epaper_admin_node.module
--------------------------------------------------------------------------------
FOUND 12 ERROR(S) AFFECTING 12 LINE(S)
--------------------------------------------------------------------------------
 20 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
    |       | question marks
 23 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
    |       | question marks
 26 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
    |       | question marks
 32 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
    |       | question marks
 39 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
    |       | question marks
 40 | ERROR | Variable "epapergroupList" is camel caps format. do not use mixed
    |       | case (camelCase), use lower case and _
 42 | ERROR | Variable "epapergroupList" is camel caps format. do not use mixed
    |       | case (camelCase), use lower case and _
 43 | ERROR | Variable "epapergroupList" is camel caps format. do not use mixed
    |       | case (camelCase), use lower case and _
 46 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
    |       | question marks
 57 | ERROR | Inline comments must end in  full-stops, exclamation marks, or
    |       | question marks
 69 | ERROR | Variable "epapergroupList" is camel caps format. do not use mixed
    |       | case (camelCase), use lower case and _
 73 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: .../sites/all/modules/pareview_temp/test_candidate/js/epaper_admin_load.js
--------------------------------------------------------------------------------
FOUND 5 ERROR(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
  5 | ERROR | Whitespace found at end of line
  9 | ERROR | Whitespace found at end of line
 35 | ERROR | Whitespace found at end of line
 53 | ERROR | Whitespace found at end of line
 55 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

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

klausi’s picture

@frob: please don't post the full output of pareview.sh inline in the comment, better add it as .txt file attachment.

frob’s picture

@klausi, I will follow your advise about the automated review. However, in my review I always preferred when the auto review was included inline. It made it easier to go line by line and fix the issues.

nmudgal’s picture

Status: Needs review » Needs work

Hi,
Please check the result of automated review here http://ventral.org/pareview/httpgitdrupalorgsandbox1000graddigital130239... & do the needful.
Manual Review:

  • epaper_admin.info : line no 3 in package it should say "Module" than "Modul", I think.
  • epaper_admin.install: Module no longer should explicitly install or uninstall its database schema in hook_install() or hook_uninstall(). http://drupal.org/node/224333#install-schema
  • Wrap title & description inside t() in this function epaper_admin_filter_info() , epaper_admin_node_form_alter(), epaper_admin_form_add_edit()
  • I think you should use this fapi #machine_name form element instead of defining textfield for machine_name in form epaper_admin_form_add_edit() http://api.drupal.org/api/drupal/developer!topics!forms_api_reference.ht...
  • check user input url with valid_url in epaper_admin_form_add_edit_validate()

Thanks.

1000.grad.digital’s picture

Status: Needs work » Needs review
frob’s picture

Status: Needs review » Needs work

If any work has been done since comment#11 I cannot see it. Did you forget to git push?

There has been no update in the code since July 5th.

Also, is this a solo or a group account. Remember that group accounts are not allowed.

This process is only for individuals.

klausi’s picture

Status: Needs work » Closed (won't fix)

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