The Views Slideshow: ImageFlow Advanced module adds a Views display for showing rows as items in ImageFlow slideshow (http://finnrudolph.de/ImageFlow).

This module is a advanced and updated version of original Views Slideshow: ImageFlow Advanced module
by aaron. This module has more configurable options and actively maintained. This module was completely rewritten and optimized.

Drupal 7 version is full release.

ImageFlow is not distributed under GPL and is not free, therefore it is not included with this module.
You will need to download it separately.

This is a views 3 and views_slideshow 3 module.
It will not work with lower versions.

Git:
alex.designworks@git.drupal.org:sandbox/alex.designworks/1192646.git

Project page:
http://drupal.org/sandbox/alex.designworks/1192646

Drupal 7 - RC1

Comments

raynimmo’s picture

raynimmo commented
Status: Needs review » Needs work

Initially I noticed the slight error in your branch naming conventions as they should not include the '-dev' tag as yet as these should be used once your module has been approved and used for the development versions. You should tag them as 6.x-1.x and 7.x-1.x

Automated review
Please keep in mind that this is primarily a high level check that does not replace but eases the review process. There is no guarantee that no other issues could show up in a more in-depth manual follow-up review.

Review of your 6.x-1.x Branch

Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):


FILE: ...lideshow_imageflow_advanced-d6\js\views_slideshow_imageflow_advanced.js
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 1 | ERROR | End of line character is invalid; expected "\n" but found "\r\n"
--------------------------------------------------------------------------------


FILE: ...ault\modules\reviews\views_slideshow_imageflow_advanced-d6\reflect3.php
--------------------------------------------------------------------------------
FOUND 67 ERROR(S) AND 6 WARNING(S) AFFECTING 40 LINE(S)
--------------------------------------------------------------------------------
   1 | ERROR   | End of line character is invalid; expected "\n" but found
     |         | "\r\n"
   2 | ERROR   | You must use "/**" style comments for a file comment
  33 | ERROR   | No space before comment text; expected "// 	PHP Version sanity
     |         | check" but found "//	PHP Version sanity check"
  33 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
  39 | ERROR   | No space before comment text; expected "// 	GD check" but
     |         | found "//	GD check"
  39 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
  45 | ERROR   | 4 spaces found before inline comment; expected "// GD Version
     |         | check" but found "//    GD Version check"
  45 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
  58 | ERROR   | No space before comment text; expected "// 	Our allowed query
     |         | string parameters" but found "//	Our allowed query string
     |         | parameters"
  59 | ERROR   | 2 spaces found before inline comment; expected "// To cache or
     |         | not to cache? that is the question" but found "//  To cache or
     |         | not to cache? that is the question"
  59 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
  74 | ERROR   | No space before comment text; expected "// 	img (the image to
     |         | reflect)" but found "//	img (the image to reflect)"
  74 | ERROR   | Inline comments must start with a capital letter
  93 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 111 | ERROR   | 4 spaces found before inline comment; expected "// tint (the
     |         | colour used for the tint, defaults to white if not given)" but
     |         | found "//    tint (the colour used for the tint, defaults to
     |         | white if not given)"
 111 | ERROR   | Inline comments must start with a capital letter
 111 | ERROR   | There must be no blank line following an inline comment
 119 | ERROR   | 4 spaces found before inline comment; expected "// Extract the
     |         | hex colour" but found "//    Extract the hex colour"
 119 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 122 | ERROR   | 4 spaces found before inline comment; expected "// Does it
     |         | start with a hash? If so then strip it" but found "//    Does
     |         | it start with a hash? If so then strip it"
 122 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 142 | ERROR   | 4 spaces found before inline comment; expected "// Wrong
     |         | values passed, default to white" but found "//    Wrong values
     |         | passed, default to white"
 142 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 149 | ERROR   | No space before comment text; expected "// 	height (how tall
     |         | should the reflection be?)" but found "//	height (how tall
     |         | should the reflection be?)"
 149 | ERROR   | Inline comments must start with a capital letter
 153 | ERROR   | No space before comment text; expected "// 	Have they given us
     |         | a percentage?" but found "//	Have they given us a percentage?"
 155 | ERROR   | No space before comment text; expected "// 	Yes, remove the %
     |         | sign" but found "//	Yes, remove the % sign"
 155 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 158 | ERROR   | No space before comment text; expected "// 	Gotta love auto
     |         | type casting ;)" but found "//	Gotta love auto type casting
     |         | ;)"
 174 | ERROR   | No space before comment text; expected "// 	No height was
     |         | given, so default to 50% of the source images height" but
     |         | found "//	No height was given, so default to 50% of the source
     |         | images height"
 174 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 218 | ERROR   | No space before comment text; expected "// 	How big is the
     |         | image?" but found "//	How big is the image?"
 232 | ERROR   | No space before comment text; expected "// 	Calculate the
     |         | height of the output image" but found "//	Calculate the height
     |         | of the output image"
 232 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 234 | ERROR   | No space before comment text; expected "// 	The output height
     |         | is a percentage" but found "//	The output height is a
     |         | percentage"
 234 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 238 | ERROR   | No space before comment text; expected "// 	The output height
     |         | is a fixed pixel value" but found "//	The output height is a
     |         | fixed pixel value"
 238 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 242 | WARNING | Line exceeds 80 characters; contains 113 characters
 242 | ERROR   | No space before comment text; expected "// 	Detect the source
     |         | image format - only GIF, JPEG and PNG are supported. If you
     |         | need more, extend this yourself." but found "//	Detect the
     |         | source image format - only GIF, JPEG and PNG are supported. If
     |         | you need more, extend this yourself."
 245 | ERROR   | No space before comment text; expected "// 	GIF" but found
     |         | "//	GIF"
 245 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 250 | ERROR   | No space before comment text; expected "// 	JPG" but found
     |         | "//	JPG"
 250 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 255 | ERROR   | No space before comment text; expected "// 	PNG" but found
     |         | "//	PNG"
 255 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 273 | ERROR   | No space before comment text; expected "// 	We'll store the
     |         | final reflection in $output. $buffer is for internal use." but
     |         | found "//	We'll store the final reflection in $output. $buffer
     |         | is for internal use."
 277 | WARNING | Line exceeds 80 characters; contains 88 characters
 277 | ERROR   | 2 spaces found before inline comment; expected "// Save any
     |         | alpha data that might have existed in the source image and
     |         | disable blending" but found "//  Save any alpha data that
     |         | might have existed in the source image and disable blending"
 277 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 286 | ERROR   | No space before comment text; expected "// 	Copy the
     |         | bottom-most part of the source image into the output" but
     |         | found "//	Copy the bottom-most part of the source image into
     |         | the output"
 286 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 289 | ERROR   | No space before comment text; expected "// 	Rotate and flip it
     |         | (strip flip method)" but found "//	Rotate and flip it (strip
     |         | flip method)"
 302 | WARNING | Line exceeds 80 characters; contains 83 characters
 302 | ERROR   | No space before comment text; expected "// 	This is quite
     |         | simple really. There are 127 available levels of alpha, so we
     |         | just" but found "//	This is quite simple really. There are 127
     |         | available levels of alpha, so we just"
 303 | WARNING | Line exceeds 80 characters; contains 88 characters
 303 | ERROR   | No space before comment text; expected "// 	step-through the
     |         | reflected image, drawing a box over the top, with a set alpha
     |         | level." but found "//	step-through the reflected image,
     |         | drawing a box over the top, with a set alpha level."
 304 | ERROR   | No space before comment text; expected "// 	The end result? A
     |         | cool fade." but found "//	The end result? A cool fade."
 305 | WARNING | Line exceeds 80 characters; contains 90 characters
 305 | ERROR   | No space before comment text; expected "// 	There are a
     |         | maximum of 127 alpha fade steps we can use, so work out the
     |         | alpha step rate" but found "//	There are a maximum of 127
     |         | alpha fade steps we can use, so work out the alpha step rate"
 305 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 305 | ERROR   | There must be no blank line following an inline comment
 312 | ERROR   | 2 spaces found before inline comment; expected "// Get % of
     |         | reflection height" but found "//  Get % of reflection height"
 312 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 315 | ERROR   | 2 spaces found before inline comment; expected "// Get % of
     |         | alpha" but found "//  Get % of alpha"
 315 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 323 | ERROR   | 2 spaces found before inline comment; expected "// Rejig it
     |         | because of the way in which the image effect overlay works"
     |         | but found "//  Rejig it because of the way in which the image
     |         | effect overlay works"
 323 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 326 | WARNING | Line exceeds 80 characters; contains 116 characters
 326 | ERROR   | No space before comment text; expected "//
     |         | imagefilledrectangle($output, 0, $y, $width, $y,
     |         | imagecolorallocatealpha($output, 127, 127, 127,
     |         | $final_alpha));" but found "//imagefilledrectangle($output, 0,
     |         | $y, $width, $y, imagecolorallocatealpha($output, 127, 127,
     |         | 127, $final_alpha));"
 356 | ERROR   | No space before comment text; expected "// 	PNG" but found
     |         | "//	PNG"
 356 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 360 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
--------------------------------------------------------------------------------


FILE: ..._imageflow_advanced-d6\theme\views-slideshow-imageflow-advanced.tpl.php
--------------------------------------------------------------------------------
FOUND 3 ERROR(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
  1 | ERROR | End of line character is invalid; expected "\n" but found "\r\n"
  4 | ERROR | The second line in the file doc comment must be " * @file"
 28 | ERROR | Concat operator must be surrounded by spaces
--------------------------------------------------------------------------------


FILE: ...mageflow_advanced-d6\theme\views_slideshow_imageflow_advanced.theme.inc
--------------------------------------------------------------------------------
FOUND 9 ERROR(S) AND 2 WARNING(S) AFFECTING 11 LINE(S)
--------------------------------------------------------------------------------
  1 | ERROR   | End of line character is invalid; expected "\n" but found
    |         | "\r\n"
  4 | ERROR   | The second line in the file doc comment must be " * @file"
  5 | WARNING | Line exceeds 80 characters; contains 84 characters
 49 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
    |         | question marks
 55 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
    |         | question marks
 63 | WARNING | Line exceeds 80 characters; contains 81 characters
 64 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
    |         | question marks
 81 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
    |         | question marks
 85 | ERROR   | Whitespace found at end of line
 93 | ERROR   | Expected "if (...) {\n"; found "if (...){\n"
 97 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
    |         | question marks
--------------------------------------------------------------------------------


FILE: ...deshow_imageflow_advanced-d6\views_slideshow_imageflow_advanced.install
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AND 2 WARNING(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
  1 | ERROR   | End of line character is invalid; expected "\n" but found
    |         | "\r\n"
  9 | WARNING | Format should be * Implements hook_foo().
 16 | WARNING | Format should be * Implements hook_foo().
--------------------------------------------------------------------------------


FILE: ...ideshow_imageflow_advanced-d6\views_slideshow_imageflow_advanced.module
--------------------------------------------------------------------------------
FOUND 4 ERROR(S) AND 4 WARNING(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
  1 | ERROR   | End of line character is invalid; expected "\n" but found
    |         | "\r\n"
  5 | WARNING | Line exceeds 80 characters; contains 91 characters
  7 | WARNING | Line exceeds 80 characters; contains 91 characters
 13 | WARNING | Line exceeds 80 characters; contains 84 characters
 69 | WARNING | Line exceeds 80 characters; contains 92 characters
 69 | ERROR   | Inline comments must start with a capital letter
 69 | ERROR   | There must be no blank line following an inline comment
 69 | ERROR   | Comments may not appear after statements.
--------------------------------------------------------------------------------


FILE: ...flow_advanced-d6\views_slideshow_imageflow_advanced.views_slideshow.inc
--------------------------------------------------------------------------------
FOUND 5 ERROR(S) AND 2 WARNING(S) AFFECTING 6 LINE(S)
--------------------------------------------------------------------------------
   1 | ERROR   | End of line character is invalid; expected "\n" but found
     |         | "\r\n"
   4 | ERROR   | The second line in the file doc comment must be " * @file"
  10 | ERROR   | You must use "/**" style comments for a function comment
  15 | ERROR   | Empty array declaration must have no space between the
     |         | parentheses
  17 | ERROR   | Empty array declaration must have no space between the
     |         | parentheses
 202 | WARNING | Line exceeds 80 characters; contains 94 characters
 202 | WARNING | Format should be * Implements hook_foo().
--------------------------------------------------------------------------------

3rd Party Code

There appears to be 3rd party code within your module. 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 that 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.

The file(s) in question are reflect3.php and while it doesnt actually carry a copyright and states within that it is free for use this file would be subject to the GPL licence that Drupal operates under. Possibly contact the author asking if this is possible/allowed or remove it and inform users that they need to download it from the authors website and install in the appropriate location.


Manual Review

  • Lines in your README.txt should not exceed 80 characters
  • The version tag is not required in your .info file

Having said all of the above, can I commend you on your inline code commenting, very thorough and explains what is going on throughout the process, well done.

Review of your 7.x-1.x Branch

Generally the same applies to your 7.x-1.x branch with regards to the 3rd party code and the notes listed in the manual review above.

There are however a few more lines that are being caught by the automated reviewer. I will echo these results here for your perusal.

Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):


FILE: ...lideshow_imageflow_advanced-d7\js\views_slideshow_imageflow_advanced.js
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 1 | ERROR | End of line character is invalid; expected "\n" but found "\r\n"
--------------------------------------------------------------------------------


FILE: ...ault\modules\reviews\views_slideshow_imageflow_advanced-d7\reflect3.php
--------------------------------------------------------------------------------
FOUND 67 ERROR(S) AND 6 WARNING(S) AFFECTING 40 LINE(S)
--------------------------------------------------------------------------------
   1 | ERROR   | End of line character is invalid; expected "\n" but found
     |         | "\r\n"
   2 | ERROR   | You must use "/**" style comments for a file comment
  33 | ERROR   | No space before comment text; expected "// 	PHP Version sanity
     |         | check" but found "//	PHP Version sanity check"
  33 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
  39 | ERROR   | No space before comment text; expected "// 	GD check" but
     |         | found "//	GD check"
  39 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
  45 | ERROR   | 4 spaces found before inline comment; expected "// GD Version
     |         | check" but found "//    GD Version check"
  45 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
  58 | ERROR   | No space before comment text; expected "// 	Our allowed query
     |         | string parameters" but found "//	Our allowed query string
     |         | parameters"
  59 | ERROR   | 2 spaces found before inline comment; expected "// To cache or
     |         | not to cache? that is the question" but found "//  To cache or
     |         | not to cache? that is the question"
  59 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
  74 | ERROR   | No space before comment text; expected "// 	img (the image to
     |         | reflect)" but found "//	img (the image to reflect)"
  74 | ERROR   | Inline comments must start with a capital letter
  93 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 111 | ERROR   | 4 spaces found before inline comment; expected "// tint (the
     |         | colour used for the tint, defaults to white if not given)" but
     |         | found "//    tint (the colour used for the tint, defaults to
     |         | white if not given)"
 111 | ERROR   | Inline comments must start with a capital letter
 111 | ERROR   | There must be no blank line following an inline comment
 119 | ERROR   | 4 spaces found before inline comment; expected "// Extract the
     |         | hex colour" but found "//    Extract the hex colour"
 119 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 122 | ERROR   | 4 spaces found before inline comment; expected "// Does it
     |         | start with a hash? If so then strip it" but found "//    Does
     |         | it start with a hash? If so then strip it"
 122 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 142 | ERROR   | 4 spaces found before inline comment; expected "// Wrong
     |         | values passed, default to white" but found "//    Wrong values
     |         | passed, default to white"
 142 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 149 | ERROR   | No space before comment text; expected "// 	height (how tall
     |         | should the reflection be?)" but found "//	height (how tall
     |         | should the reflection be?)"
 149 | ERROR   | Inline comments must start with a capital letter
 153 | ERROR   | No space before comment text; expected "// 	Have they given us
     |         | a percentage?" but found "//	Have they given us a percentage?"
 155 | ERROR   | No space before comment text; expected "// 	Yes, remove the %
     |         | sign" but found "//	Yes, remove the % sign"
 155 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 158 | ERROR   | No space before comment text; expected "// 	Gotta love auto
     |         | type casting ;)" but found "//	Gotta love auto type casting
     |         | ;)"
 174 | ERROR   | No space before comment text; expected "// 	No height was
     |         | given, so default to 50% of the source images height" but
     |         | found "//	No height was given, so default to 50% of the source
     |         | images height"
 174 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 218 | ERROR   | No space before comment text; expected "// 	How big is the
     |         | image?" but found "//	How big is the image?"
 232 | ERROR   | No space before comment text; expected "// 	Calculate the
     |         | height of the output image" but found "//	Calculate the height
     |         | of the output image"
 232 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 234 | ERROR   | No space before comment text; expected "// 	The output height
     |         | is a percentage" but found "//	The output height is a
     |         | percentage"
 234 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 238 | ERROR   | No space before comment text; expected "// 	The output height
     |         | is a fixed pixel value" but found "//	The output height is a
     |         | fixed pixel value"
 238 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 242 | WARNING | Line exceeds 80 characters; contains 113 characters
 242 | ERROR   | No space before comment text; expected "// 	Detect the source
     |         | image format - only GIF, JPEG and PNG are supported. If you
     |         | need more, extend this yourself." but found "//	Detect the
     |         | source image format - only GIF, JPEG and PNG are supported. If
     |         | you need more, extend this yourself."
 245 | ERROR   | No space before comment text; expected "// 	GIF" but found
     |         | "//	GIF"
 245 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 250 | ERROR   | No space before comment text; expected "// 	JPG" but found
     |         | "//	JPG"
 250 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 255 | ERROR   | No space before comment text; expected "// 	PNG" but found
     |         | "//	PNG"
 255 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 273 | ERROR   | No space before comment text; expected "// 	We'll store the
     |         | final reflection in $output. $buffer is for internal use." but
     |         | found "//	We'll store the final reflection in $output. $buffer
     |         | is for internal use."
 277 | WARNING | Line exceeds 80 characters; contains 88 characters
 277 | ERROR   | 2 spaces found before inline comment; expected "// Save any
     |         | alpha data that might have existed in the source image and
     |         | disable blending" but found "//  Save any alpha data that
     |         | might have existed in the source image and disable blending"
 277 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 286 | ERROR   | No space before comment text; expected "// 	Copy the
     |         | bottom-most part of the source image into the output" but
     |         | found "//	Copy the bottom-most part of the source image into
     |         | the output"
 286 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 289 | ERROR   | No space before comment text; expected "// 	Rotate and flip it
     |         | (strip flip method)" but found "//	Rotate and flip it (strip
     |         | flip method)"
 302 | WARNING | Line exceeds 80 characters; contains 83 characters
 302 | ERROR   | No space before comment text; expected "// 	This is quite
     |         | simple really. There are 127 available levels of alpha, so we
     |         | just" but found "//	This is quite simple really. There are 127
     |         | available levels of alpha, so we just"
 303 | WARNING | Line exceeds 80 characters; contains 88 characters
 303 | ERROR   | No space before comment text; expected "// 	step-through the
     |         | reflected image, drawing a box over the top, with a set alpha
     |         | level." but found "//	step-through the reflected image,
     |         | drawing a box over the top, with a set alpha level."
 304 | ERROR   | No space before comment text; expected "// 	The end result? A
     |         | cool fade." but found "//	The end result? A cool fade."
 305 | WARNING | Line exceeds 80 characters; contains 90 characters
 305 | ERROR   | No space before comment text; expected "// 	There are a
     |         | maximum of 127 alpha fade steps we can use, so work out the
     |         | alpha step rate" but found "//	There are a maximum of 127
     |         | alpha fade steps we can use, so work out the alpha step rate"
 305 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 305 | ERROR   | There must be no blank line following an inline comment
 312 | ERROR   | 2 spaces found before inline comment; expected "// Get % of
     |         | reflection height" but found "//  Get % of reflection height"
 312 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 315 | ERROR   | 2 spaces found before inline comment; expected "// Get % of
     |         | alpha" but found "//  Get % of alpha"
 315 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 323 | ERROR   | 2 spaces found before inline comment; expected "// Rejig it
     |         | because of the way in which the image effect overlay works"
     |         | but found "//  Rejig it because of the way in which the image
     |         | effect overlay works"
 323 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 326 | WARNING | Line exceeds 80 characters; contains 116 characters
 326 | ERROR   | No space before comment text; expected "//
     |         | imagefilledrectangle($output, 0, $y, $width, $y,
     |         | imagecolorallocatealpha($output, 127, 127, 127,
     |         | $final_alpha));" but found "//imagefilledrectangle($output, 0,
     |         | $y, $width, $y, imagecolorallocatealpha($output, 127, 127,
     |         | 127, $final_alpha));"
 356 | ERROR   | No space before comment text; expected "// 	PNG" but found
     |         | "//	PNG"
 356 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
 360 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
     |         | question marks
--------------------------------------------------------------------------------


FILE: ..._imageflow_advanced-d7\theme\views-slideshow-imageflow-advanced.tpl.php
--------------------------------------------------------------------------------
FOUND 3 ERROR(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
  1 | ERROR | End of line character is invalid; expected "\n" but found "\r\n"
  4 | ERROR | The second line in the file doc comment must be " * @file"
 28 | ERROR | Concat operator must be surrounded by spaces
--------------------------------------------------------------------------------


FILE: ...mageflow_advanced-d7\theme\views_slideshow_imageflow_advanced.theme.inc
--------------------------------------------------------------------------------
FOUND 14 ERROR(S) AND 2 WARNING(S) AFFECTING 15 LINE(S)
--------------------------------------------------------------------------------
  1 | ERROR   | End of line character is invalid; expected "\n" but found
    |         | "\r\n"
  4 | ERROR   | The second line in the file doc comment must be " * @file"
  5 | WARNING | Line exceeds 80 characters; contains 84 characters
 40 | ERROR   | Whitespace found at end of line
 50 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
    |         | question marks
 54 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
    |         | question marks
 62 | WARNING | Line exceeds 80 characters; contains 81 characters
 63 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
    |         | question marks
 80 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
    |         | question marks
 84 | ERROR   | Whitespace found at end of line
 88 | ERROR   | Concat operator must be surrounded by spaces
 88 | ERROR   | Concat operator must be surrounded by spaces
 92 | ERROR   | Expected "if (...) {\n"; found "if (...){\n"
 94 | ERROR   | Whitespace found at end of line
 95 | ERROR   | Whitespace found at end of line
 96 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
    |         | question marks
--------------------------------------------------------------------------------


FILE: ...ideshow_imageflow_advanced-d7\views_slideshow_imageflow_advanced.module
--------------------------------------------------------------------------------
FOUND 4 ERROR(S) AND 4 WARNING(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
  1 | ERROR   | End of line character is invalid; expected "\n" but found
    |         | "\r\n"
  5 | WARNING | Line exceeds 80 characters; contains 91 characters
  7 | WARNING | Line exceeds 80 characters; contains 91 characters
 13 | WARNING | Line exceeds 80 characters; contains 84 characters
 50 | WARNING | Line exceeds 80 characters; contains 92 characters
 50 | ERROR   | Inline comments must start with a capital letter
 50 | ERROR   | There must be no blank line following an inline comment
 50 | ERROR   | Comments may not appear after statements.
--------------------------------------------------------------------------------


FILE: ...flow_advanced-d7\views_slideshow_imageflow_advanced.views_slideshow.inc
--------------------------------------------------------------------------------
FOUND 12 ERROR(S) AND 2 WARNING(S) AFFECTING 13 LINE(S)
--------------------------------------------------------------------------------
   1 | ERROR   | End of line character is invalid; expected "\n" but found
     |         | "\r\n"
   4 | ERROR   | The second line in the file doc comment must be " * @file"
  10 | ERROR   | You must use "/**" style comments for a function comment
  15 | ERROR   | Empty array declaration must have no space between the
     |         | parentheses
  17 | ERROR   | Empty array declaration must have no space between the
     |         | parentheses
 145 | ERROR   | A whitespace must follow to the item assignemtn operator =>
 148 | ERROR   | Whitespace found at end of line
 149 | ERROR   | Whitespace found at end of line
 156 | ERROR   | A whitespace must follow to the item assignemtn operator =>
 159 | ERROR   | Whitespace found at end of line
 163 | ERROR   | Whitespace found at end of line
 164 | ERROR   | Whitespace found at end of line
 216 | WARNING | Line exceeds 80 characters; contains 94 characters
 216 | WARNING | Format should be * Implements hook_foo().
--------------------------------------------------------------------------------

Please ensure that you set your projecxts staus back to 'needs review' when you have addressed the issues here.

Good luck with the rest of your review.

alex.skrypnyk’s picture

alex.skrypnyk
he/him
Primary language English
Location Melbourne
commented
Status: Needs work » Needs review

All issues were fixed.

6.x branch was removed as it is no longer supported.

As for 3rd party code - I've contacted the developer and most likely that code will be released under GPL. I will post back here once I here from here. In worst case scenario, I will rewrite that code and release it under GPL.

alex.skrypnyk’s picture

alex.skrypnyk
he/him
Primary language English
Location Melbourne
commented
Issue summary: View changes

Removed description of 6.x branch

raynimmo’s picture

raynimmo commented

Maybe just look into adding it through the Libraries API, it is a fairly straightforward procedure. By using the Libraries API then it will allow for any other modules that use the same piece of code to function more effectively without the possiblity of any namespace clashes.

alex.skrypnyk’s picture

alex.skrypnyk
he/him
Primary language English
Location Melbourne
commented

I know how to use Library API. Indeed, Imageflow JS is linked using Libraries API.

The file reflect3.php is not original reflect.php script. I had to modify it to allow working with PHP 5 and have done some extra cleaning up. If I put it into libraries - it has to be downloaded from somewhere, right? But where can it be downloaded from if it is customized script? I can not host someone's script on my personal server.

So, I guess I will wait until the author will come back to me within next 3-5 days. If not - I will either remove this functionality or rewrite the script.

bfr’s picture

bfr commented
Status: Needs review » Needs work

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

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.

alex.skrypnyk’s picture

alex.skrypnyk
he/him
Primary language English
Location Melbourne
commented
Status: Needs work » Needs review

Formatting fixed.

Please review.

broncomania’s picture

broncomania commented

Will there be a version for D6 or is it deprecated? Why do you remove that?

patrickd’s picture

patrickd commented
Status: Needs review » Needs work

Still some formatting issues: http://ventral.org/pareview/httpgitdrupalorgsandboxalexdesignworks119264...

alex.skrypnyk’s picture

alex.skrypnyk
he/him
Primary language English
Location Melbourne
commented
Status: Needs work » Needs review

All formatting issues fixed (checked at http://ventral.org/pareview).

@broncomania
D6 version will be back-ported as soon as this project published.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Closed (duplicate)

Please only post one project application, you already have one in #1162758: Menu columns. We will approve you only once which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Marking this one as duplicate.

alex.skrypnyk’s picture

alex.skrypnyk
he/him
Primary language English
Location Melbourne
commented
Status: Closed (duplicate) » Needs review

I've closed the other module issue and re-opened this one.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work
  • "define('IMAGEFLOW_PACKED', TRUE);": all constants should be prefixed with your module's name.
  • "if (module_exists('libraries')) {": your module depends on libraries API, so you do not have to check that.
  • views_slideshow_imageflow_advanced_views_slideshow_options_form_validate(): why do you use the "!" placeholder? I gues the setting titles are all plain text, so you should use "@".
  • IMAGEFLOW_PACKED: Isn't this a setting a user would want to change?
alex.skrypnyk’s picture

alex.skrypnyk
he/him
Primary language English
Location Melbourne
commented
Status: Needs work » Needs review

@klausi
Thanks for pointing those issues. They have been fixed along with other small issue.

prashantgoel’s picture

prashantgoel commented
Status: Needs review » Needs work

please visit http://ventral.org/pareview/httpgitdrupalorgsandboxalexdesignworks119264... for the list of errors being generated.

patrickd’s picture

patrickd commented
Status: Needs work » Needs review

don't block deeper reviews because of minor issues found by automated reviews

exlin’s picture

exlin commented

I understand that this module is rewrite of http://drupal.org/project/views_slideshow_imageflow and has mode configuration options.

Have you considered joining forces with aaron to make existing one better or requesting rights for that project if it is currently unmaintained?

alex.skrypnyk’s picture

alex.skrypnyk
he/him
Primary language English
Location Melbourne
commented

This module is not a rewrite, but a totally new implementation of http://drupal.org/project/views_slideshow_imageflow, with more functionality.

It does not provide any backwards compatibility with views_slideshow_imageflow.

This module is intended to stay independent from views_slideshow_imageflow, as it provide more features.

sylvain lecoy’s picture

sylvain lecoy commented

Manual review:

Code is correct. However, your module should specify in its info file that libraries 7.x-1.0 is needed and not the 2.0 version.

This can be done by adding to the line:

dependencies[] = libraries (<2.0)

More about this here: http://drupal.org/node/542202#dependencies

You distinguished the preprocess function from theme layer, that's a good thing. Also you are making use of Drupal.behaviors for javascript so it seems you have a good knowledge in general.

I don't know if I am allowed to RTBC your application.

patrickd’s picture

patrickd commented

anyone can set RTBC if you think it is

sylvain lecoy’s picture

sylvain lecoy commented
Status: Needs review » Reviewed & tested by the community

It seems that I can.

misc’s picture

misc commented
Status: Reviewed & tested by the community » Postponed (maintainer needs more info)

Sorry, but I need to set this as postponed - I think the status of the reflect3.php script included need to be solved before we can take this further.
If you included a script, it should be clear it is GPL.

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented

If #1363326: Replace reflection3.php with JS solution got implemented, could we remove reflect3.php altogether, and thus unblock this module from being accepted as fully-fledged?

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Postponed (maintainer needs more info) » Closed (won't fix)

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

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Issue summary: View changes

Description updated

nwom’s picture

nwom commented
Issue summary: View changes

The sandbox page is sadly 404. Is there a possibility that this project will ever come to life, especially since it is no longer required to make sandbox modules first? Having a VIews Slideshow implementation of Imageflow is sorely needed for D7 still. Thanks for everyone's work on this.

Also, does anyone have any way of accessing the older sandbox files in the interim?

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Views Slideshow: Imageflow Advanced » [D7] Views Slideshow: Imageflow Advanced
Issue tags: -views, -slider, -imageflow, -3d views carousel, -views_slideshow