jQuery carousel is based on the jquery plugin http://github.com/richardscarrott/jquery-ui-carousel. It allows developers to create carousels using the data entered through the content types. The carousels could be vertical or Horizontal depending on the configurations. These carousels can be configured for "swipe to slide" on mobile devices as well. Also, the module allows us to have "multiple carousels" with different configurations on a single page as it ties up the config for a carousel to its instance only rather than having it as a global config for the page.

Demo: http://jcarousel.qed42.webfactional.com/jquery-carousel-demo

What this module Provides:

  • Views style Plugin: This module provides a views style plugin to display the view-rows in the form of a carousel.
  • Field Formatter Plugin: This module provides a field formatter plugin for image field. Displays the multi-valued content as a carousel.
  • hook_carousel_theme_info(): This hooks allows developers to register a new theme for the carousel
    and load the corresponding CSS file. e.g.,
    /**
         * Implements hook_carousel_them_info()
         */
        function mymodule_carousel_theme_info() {
          $themes = array();
    
          $themes['my_custom_theme'] = array(
            'title' => t('My Custom Theme'),
            'file' => 'my_module/my_custom_theme/jquery-carousel-default.css'
          );
    
          return $themes;
        }
  • Configurable vertical & Horizontal carousels.
  • Touch support for mobile devices
  • Can easily place multiple carousels with different configurations on a single page. Carousel configurations are tied up with carousels & not the page.


Requirements:


This module depends on jquery ui carousel library(http://github.com/richardscarrott/jquery-ui-carousel).

Installation:

  • Download the module and place it with other contributed modules (e.g. sites/all/modules/contrib).
  • Enable the jQuery Carousel module on the Modules list page.
  • Download the jquery ui carousel library form http://github.com/richardscarrott/jquery-ui-carousel and save it in sites/all/libraries folder.
  • Goto admin/reports/status & make sure jquery ui carousel is not throwing errors.


Usage:

  • Views Style Plugin:
    • Goto admin/structure/views. Click on "Add new view"
    • Create a view with "page" display. Add fields you need to be displayed in a carousel. This would
      mostly be images. Add images with appropriate "image presets".
    • Change the "Format" to jQuery Carousel and click on "Apply this display".
    • Configure the plugin in the settings form that pops up.
    • Save the view and visit the path set for the view while creating the page display.
    • Save the view and visit the path set for the view while creating the page display.
    • You should be able to see the data requested in the view in the form of a carousel.
      Field Formatter Plugin:
    • Goto admin/structure/types. You should be able to see article content type(ships with an image field).
    • Click on Manage display.
    • Under format select list, choose jQuery Carousel. You will see the same settings form as you see for the
      views style plugin. Configure it as per the requirements.
    • Goto node/add/article. Create an article with multiple images uploaded.
    • The view page for the article should show the multiple values in a carousel.

NOTE:
a. Module ships with only one default theme named as default. You can create as many themes as
needed using the hook explained under What this module Provides#3.
b. You must add a selector value in the selector field coming up in settings form. This is the
class that the carousel's markup gets wrapped with & the configs done get applied to.


How is it different from other carousels:

  • Allows UI developers to write CSS for it rather than overrriding the CSS that comes right from the plugin
  • Provides an option to have vertical & horizontal carousels
  • Provides touch mode configuration for mobile devices
  • Provides field formatter for multi-valued fields

Link to Project:-
https://www.drupal.org/sandbox/piyuesh23/2336009

Link to git Repository:-
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/piyuesh23/2336009.git jquery_carousel

PAReview url
http://pareview.sh/pareview/httpgitdrupalorgsandboxpiyuesh232336009git

Reviews of other projects

Comments

piyuesh23’s picture

Issue summary: View changes
piyuesh23’s picture

Issue summary: View changes
piyuesh23’s picture

Status: Active » Needs review
piyuesh23’s picture

Issue summary: View changes
PA robot’s picture

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

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

piyuesh23’s picture

Issue summary: View changes
piyuesh23’s picture

Issue tags: +PAReview, +PAReview: review bonus
dbt102’s picture

Hi @piyuesh23,
Thank you for contributing your project to Drupal.

I have reviewed your application per the review checklist and post these below for your reference. I know that not all new contributors have the knowledge to successfully follow all of the points it covers from scratch, but the more of them you can resolve now, the less reviewers have to remind you later and the quicker your application can be approved.

1.1 Your application contains a repository and project page link, however you do not need to replicate the project page in your application issue. I’d rather just see a brief description of what the code is doing, then use the project page link to go there as I need.

The "git clone" command does work.

1.2 (+) I’m not so sure your project is not a duplication. Can you please explain how this is different than the many other types of carousels out there?

You should really spend some time searching for modules which provide similar functionality. As we prefer collaboration over competition we want to prevent having duplicating modules on drupal.org. If the differences between your modules are not too fundamental for patching an existing one, we would love to see you joining forces and concentrate all power on enhancing one module. (If the existing module is abandoned, please think about taking it over).

1.3 It looks like you don't have multiple applications.

2.1 (+) The repository actually contains code, however when install via git clone the module installs as directory “2336009”. I’m assuming you want it to install as jquery_carousel?

Please have a look at the documentation about using GIT on drupal.org for more info on how to fix this.

2.2 Git Status result shows “Your branch is up-to-date with ‘origin/7.x-1.x'.” So, you are working in a version specific branch, which is good.

3.1 Pareview reports no security issues. However, it does report the following …

FILE: ...eview/pareview_temp/views/plugins/jqueryCarouselPluginStyleCarousel.inc
--------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
18 | ERROR | Public method name
| | "JqueryCarouselPluginStyleCarousel::option_definition" is not in
| | lowerCamel format, it must not contain underscores
26 | ERROR | Public method name
| | "JqueryCarouselPluginStyleCarousel::options_form" is not in
| | lowerCamel format, it must not contain underscores
--------------------------------------------------------------------------------

Its been my experience that certain errors may mask others from detection in pareview. I would recommend fixing the errors indicated then running preview again to make sure there is no masking of security issues occurring in the automated review process.

Since this step is the most important one in the whole process wouldn't hurt to double check by reading through the documentation about writing secure code again.

4.1 The repository does not contain a ‘LICENSE.txt’ file.

4.2 The repository does not contain any 3rd party (non-GPL) code.

5.1 The project page contains detailed information.

5.2 The repository contains a detailed README.txt.

5.3 Your code may be on the lite side of a well-balanced amount of inline-comments.

Make sure your code is well commented and clearly understandable by other developers. Please have a look at the module documentation guidelines and the Doxygen and comment formatting conventions

6.1 (+) Upon running an automated review I find there are some issues. I have personally seen these before in other modules involving use of Views. However, I would suggest you experiment suppressing the errors and try running the automated reviews again to make sure they are not masking more significant items.

Automated reviews may point you to possible security issues - what does not mean they are really security issues - note that it's a common case that automated reviews can have false positives.

7.1 Please ensure you are using Drupes API correctly. Take a deeper dive through the project's code (line-by-line), and make sure there are no common mistakes in your code.

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.

dbt102’s picture

Status: Needs review » Needs work
piyuesh23’s picture

@dbt102,

Thanks for detailed review of the module. Will fix these and update accordingly.

piyuesh23’s picture

Issue summary: View changes
piyuesh23’s picture

Issue summary: View changes
piyuesh23’s picture

1.2> Updated the Description of this issue with another sub-heading "How is it different from other carousels:"

2.1> The description is updated with the correct git clone command to clone it as jquery_carousel.

3.1> Cannot be fixed. Here we are extending a base class views_plugin_style provided by Views module itself and overriding the two functions "options_definition" & "options_form". The names should be same for overriding.

6.1> same as 3.1

piyuesh23’s picture

Status: Needs work » Needs review
naveenvalecha’s picture

Hi @piyuesh23,
Thank you for contributing your project to Drupal.Overall, +1 for RTBC.Few nutpicks below.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder.
http://pareview.sh/pareview/httpgitdrupalorgsandboxpiyuesh232336009git

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
May be: Does not cause module duplication and fragmentation.I have googled around the carousal modules and seems that there is also https://www.drupal.org/project/field_slideshow and https://www.drupal.org/project/jcarousel Would you please specify the differences how this module is different.
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
Coding style & Drupal API usage
  1. Minor finding : line 192 and 193 .we can declare a variable in single statement like this. $form = $themes = array();
  2. Minor finding : Please remove the VERSION from the modulename.info file.Ths is automatically added by the drupal.org

This review uses the Project Application Review Template.

klausi’s picture

Status: Needs review » Needs work

Please mention the other projects and how they are different on the project page.

Why is the project page talking about Honeypot?

piyuesh23’s picture

Naveen/Klausi,

Thanks for reviewing the Project. I have already listed the points in the Description on how this project is different form other carousel modules available on Drupal.org. Quoting them here once again:


How is it different from other carousels:

  • Provides an option to have vertical & horizontal carousels
  • Provides touch mode configuration for mobile devices
  • Provides field formatter for multi-valued fields
  • Allows UI developers to write CSS for it rather than overriding the CSS that comes right from the plugin.

The first 3 points mentioned above are not provided by either of https://www.drupal.org/project/field_slideshow or https://www.drupal.org/project/jcarousel.

Removed the version from info file & updated the module file as well.

Klausi,

Updated the difference on the project page as well. Updated the D8 pledge as well.

piyuesh23’s picture

Status: Needs work » Needs review
naveenvalecha’s picture

Hi Piyuesh,
Thanks for specifying the differences and update as the requested changes in the project.
I have some thoughts below :

Provides an option to have vertical & horizontal carousels

jcarousel module also provide the option to create the vertical carousels as well.

Provides field formatter for multi-valued fields

Regarding the field vertical and horizontal jcarousal with fields its easily integerated with the field_slideshow module. http://drupal.stackexchange.com/a/52561

Provides touch mode configuration for mobile devices

IMHO,It will be very helpful if we help in maintaining existing drupal.org projects for any feature.

It will be kinda very helpful if you mention the features provided by this module are different from the existing contributed projects as I mentioned.Also please see duplication and fragmentation
I am changing the status of the issue to 'needs work'. Please feel free to change the status.
Also want to hear from klausi,what he will reckon here.

naveenvalecha’s picture

Status: Needs review » Needs work
piyuesh23’s picture

Naveen,

Thanks for highlighting these. Din't know jCarousel has a vertical carousel setting as well.

Both of these modules provide one functionality & not the other. So, for example if i need to show 2 carousels on a page: One using a single valued image field configured with views style plugin & another multivalued field with multiple images in form of a carousel, i would need to use install both these modules.

Regarding Touch support:
Its the jQuery library http://github.com/richardscarrott/jquery-ui-carousel that provides the touch support. jCarousel js Library doesn't provide an option for that. So, its not a Drupal thing to contribute back for jcarousel.

Lemme know what do you think about these.

naveenvalecha’s picture

@piyuesh23,

Both of these modules provide one functionality & not the other. So, for example if i need to show 2 carousels on a page: One using a single valued image field configured with views style plugin & another multivalued field with multiple images in form of a carousel, i would need to use install both these modules.

IMHO,I would suggest to use the existing drupal contrib modules and not to rush the more modules on the drupal.org for both features.This leads to confusion.But as I am not a git administer So I would also love to hear from klausi or another git admin.I will pm the git admins. What they will reckon here.

Regarding Touch support:
Its the jQuery library http://github.com/richardscarrott/jquery-ui-carousel that provides the touch support. jCarousel js Library doesn't provide an option for that. So, its not a Drupal thing to contribute back for jcarousel.

Regarding this we can add the feature request in the jcarousel module and add this feature in the existing module by making liasie with the maintainer and by providing support by the patches.This will be helpful for the module maintainers to add the feature in the module.

Thanks again,

piyuesh23’s picture

Naveen,

Agreed for #1, these could be patched into the existing modules.

But for #2, its not the Drupal module that needs to be patched up. Its the js library the modules use that would need to be patched up. The touch mode comes from the library we using for this module & not from the Drupal end.

Will wait for GIT admin's inputs as well.

naveenvalecha’s picture

@piuyesh23,
I have discussed regarding this case with the @mpdonadio as I did not find the klausi there because there are lot of carousal modules available on the drupal.org.So for not rushing more modules with little similar features and reduce duplication and fragmentation I would also like to hear from GIT admins here. I would love to +1 this module for the module code as per my review above as it follows the drupal coding standards.But waiting for the GIT admins to hear about the duplication.

Thanks Again !

piyuesh23’s picture

Status: Needs work » Needs review
mpdonadio’s picture

Assigned: Unassigned » mpdonadio

This sounds like a feature that should live in the existing uicarousel project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the uicarousel issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.

We can proceed with review of this project to grant vetted access, simultaneous to the above. My call is that this is not a duplicate, as I can't find another carousel module that uses this plugin or provides most of the same features (and in this case, you really want to leverage an existing JS library to keep up with touch changes).

I am going to assign to myself for a review.

mpdonadio’s picture

Assigned: mpdonadio » Unassigned
Status: Needs review » Needs work
Issue tags: -jquery plugin, -views style plugins, -Image Field Formatters, -PAReview +PAReview: security

Automated Review

Git default branch is not set, see the documentation on setting a default branch.
Review of the 7.x-1.x branch (commit 0f963d2):

  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
    
    FILE: ...t/PAR/pareview_temp/views/plugins/jqueryCarouselPluginStyleCarousel.inc
    --------------------------------------------------------------------------------
    FOUND 2 ERRORS AFFECTING 2 LINES
    --------------------------------------------------------------------------------
     18 | ERROR | Public method name
        |       | "JqueryCarouselPluginStyleCarousel::option_definition" is not in
        |       | lowerCamel format, it must not contain underscores
     26 | ERROR | Public method name
        |       | "JqueryCarouselPluginStyleCarousel::options_form" is not in
        |       | lowerCamel format, it must not contain underscores
    --------------------------------------------------------------------------------
    
    Time: 483ms; Memory: 10Mb
    
  • No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

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.

The automated test results are false positivies b/c of Views naming conventions.

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
Yes: Does not cause module duplication and fragmentation. Addressed.
Master Branch
Yes: Follows the guidelines for master branch, but still need to set the default.
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
No. If "no", list security issues identified.

In theme_jquery_carousel_field_formatter(), $vars['settings']['selector'] goes right into the output unsanitized. Look into drupal_attributes().

It looks like the same thing happens with views/theme/jquery-carousel.tpl.php

Coding style & Drupal API usage
(+) The README should should an exact path to the library JS as an example.

(+) You don't need a $(document).ready() inside the attach.

The CSS has some overly generic class names, and also used IDs (avoid when possible).

(+) Your behavior should use the context and settings that get passed to it; this makes it more AJAX friendly.

Views plugins normally don't need to be defined in the files[] in the .info.

Why do you need to explicitly include the views include? Comment needed.

In jquery_carousel_field_formatter_view(), don't call theme() and stuff it into #markup. Populate the $element with a proper render array.

Using #attached is preferred over drupal_add_library(), drupal_add_js(), and drupal_add_js().

In jquery_carousel_themes(), you have an explicit $static. See drupal_static().

(+) Your forms could use some validation.

Whay are none of the links in your hook_help() actual links? See See https://api.drupal.org/api/drupal/includes%21common.inc/function/l/7 for how to handle this in translated strings.

jquery_carousel_field_formatter_settings_summary() should probably include a better sumary of the selected options. Make sure you sanitize them. See https://www.drupal.org/node/28984

You have a handful of double quoted strings. Single quoted ones are preferred per Drupal coding standards.

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.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

piyuesh23’s picture

@mpdonadio,

Thanks for your review comments.

    Updates:
      Automated Review
    1. Set the default branch to 7.x-1.x
    2. The functions detected by PHP codesniffer are the overrides to the ones that have already been defined in the views core. Don't think we can fis this without getting the names fixed in views Core.
      Coding style & Drupal API usage
    1. README updated with full path to the library folder.

On a vacation right now and have limited access to the code. Will write simpletests for the project and fix other code related issues by 8th Oct. and assign it back for review.

mpdonadio’s picture

Cool. PAReview are suggestions, and not mandatory changes. Tests are not mandatory, either, but encouraged. The security issues are the blockers right now.

piyuesh23’s picture

Status: Needs work » Needs review

@mpdonadio,

Have fixed the issues & review comments mentioned above. Setting the state to needs review.

er.pushpinderrana’s picture

Assigned: Unassigned » er.pushpinderrana

Assigning to myself for next review.

er.pushpinderrana’s picture

Assigned: er.pushpinderrana » Unassigned
Status: Needs review » Needs work
FileSize
47.51 KB
21.97 KB

Automated Review

http://pareview.sh/pareview/httpgitdrupalorgsandboxpiyuesh232336009git reported some issues that need to be fix.

FILE: ...eview/pareview_temp/views/plugins/jqueryCarouselPluginStyleCarousel.inc
--------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
18 | ERROR | Public method name
| | "JqueryCarouselPluginStyleCarousel::option_definition" is not in
| | lowerCamel format, it must not contain underscores
26 | ERROR | Public method name
| | "JqueryCarouselPluginStyleCarousel::options_form" is not in
| | lowerCamel format, it must not contain underscores
--------------------------------------------------------------------------------

FILE: /var/www/drupal-7-pareview/pareview_temp/jquery_carousel.module
--------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
--------------------------------------------------------------------------------
130 | ERROR | Comment indentation error, expected only 1 spaces
143 | WARNING | A comma should follow the last multiline array item. Found: )
--------------------------------------------------------------------------------

FILE: /var/www/drupal-7-pareview/pareview_temp/jquery_carousel.info
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
6 | ERROR | Declared file was not found
--------------------------------------------------------------------------------

Manual Review

  • (*) I tried to test this module functionality but somehow it's throwing js error every time. Other than this, usage of drupal_attributes() makes your code secure but still for one scenario it break the js code. Input any html tag or special character in Selector for this carousel configuration field, would break the js as shown below.

    Jquey JS Break
  • (+) In views, if I open settings in format section, it throws following warning everytime that need to be address.

    Warning

  • (+)
    In jquery_carousel_field_formatter_view(), don't call theme() and stuff it into #markup. Populate the $element with a proper render array.

    Remove the commented code as you are using #markup now.

  • You have addressed most of the points mentioned in #27. Good Job!

    (+) Your forms could use some validation.

    I think addition of this would solve above issues.

    Else your module code looks good and clean to me. Only first point stopping me to move this RTBC otherwise it is ready to go.

    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.

    Thanks!

piyuesh23’s picture

@er.pushpinderrana,

Thanks for reviewing the project. Have added a validation check & restricted the selector names to be strings without any special chars or spaces. This takes care of 2nd part of #1.

I am unable to reproduce the jquery error you getting though. However, looks like its coming from the jquery library being used.

Can you please help me reproducing this? The first check would be if the file at /sites/all/libraries/jquery-ui-carousel/dist/js/jquery.rs.carousel.js is getting loaded for you or not.

piyuesh23’s picture

Status: Needs work » Needs review

Tried this multiple times within vanilla Drupal-7.32. But unable to reproduce the errors mentioned above.

Error reproducible only when the library is not available.

Setting the status to needs review so that others can check if it's reproducible on their end as well.

pgautam’s picture

Status: Needs review » Needs work

With vanilla, this module works well but as mentioned in #32.1 js break when we put any html tag or special character like (`). As this form attribute exists for content type Manage Display too, cause similar issue over there. I would recommend you to do some thing with your js code because preventing special character is not a best practice, generally hyphen(-) use in variable name. You need to make sure, js code should not break from any user input value.

piyuesh23’s picture

@pgautam,

Thanks for reviewing this. I was under the impression that the issue is coming up regardless of the selector value in config.

Added a helper function _jquery_carousel_config_validate & using the same function both in views config & field formatter config to handle the user input now. Also, have modified my selector regex to allow for '-'. I don't think other than that any other character is valid for use as a class name, so the server side validation should be fine here. Wanna avoid doing any validations on JS side that can be handled on server side..:)

Setting the status back to "needs review". Feel free to update the status of the issue as needed.

piyuesh23’s picture

Status: Needs work » Needs review
pgautam’s picture

Status: Needs review » Needs work

@piyuesh23, updated code not working at all, it shows the validation error but not preventing form submission as same issue still exists, in addition also producing some warnings as shown below.

Notice: Undefined variable: selector in jquery_carousel_config_form_validate() (line 108 of D:\xampp\htdocs\drupal\sites\all\modules\jquery_carousel\jquery_carousel.module).
Warning: Illegal offset type in isset or empty in form_set_error() (line 1623 of D:\xampp\htdocs\drupal\includes\form.inc).
Warning: Illegal offset type in form_set_error() (line 1649 of D:\xampp\htdocs\drupal\includes\form.inc).
Selector should any special characters or spaces. Only special character allowed is '-'

As I suggested earlier

I would recommend you to do some thing with your js code because preventing special character is not a best practice

IMHO, You should escape HTML entities or special character at javascript end to resolve this issue.

piyuesh23’s picture

@pgautam,

I would still like to stick with doing the validation & restricting users from adding weird characters as selectors. Also, as i mentioned above there is only one special char that gets used in class names i.e., '-' which has been added as an exception to regex now. Also, the classname set as selector gets added as the wrapper's class in theme layer while building the carousel. This might make it difficult to escape the HTML entities in JS.

The reason for the validation to allow you to submit the form despite of error was use of form_set_error instead of form_error while validating an element. Have fixed the problem now. Module wouldn't allow us to submit the form anymore with an error. The warnings above have also been fixed.

@er.pushpinderrana,

While looking at this issue i was also able to reproduce the warning you were getting in the views config section

(+) In views, if I open settings in format section, it throws following warning everytime that need to be address.

This has also been fixed now.

Setting the status back to needs review.

piyuesh23’s picture

Status: Needs work » Needs review
er.pushpinderrana’s picture

Status: Needs review » Reviewed & tested by the community

Automated Review

Review of the 7.x-1.x branch (commit c415dfc):

FILE: ...eview/pareview_temp/views/plugins/jqueryCarouselPluginStyleCarousel.inc
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
41 | WARNING | Do not use the raw $form_state['input'], use
| | $form_state['values'] instead where possible
--------------------------------------------------------------------------------
FILE: ...eview/pareview_temp/views/plugins/jqueryCarouselPluginStyleCarousel.inc
--------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
--------------------------------------------------------------------------------
18 | ERROR | Public method name
| | "JqueryCarouselPluginStyleCarousel::option_definition" is not in
| | lowerCamel format, it must not contain underscores
26 | ERROR | Public method name
| | "JqueryCarouselPluginStyleCarousel::options_form" is not in
| | lowerCamel format, it must not contain underscores
40 | ERROR | Public method name
| | "JqueryCarouselPluginStyleCarousel::options_validate" is not in
| | lowerCamel format, it must not contain underscores
--------------------------------------------------------------------------------

FILE: /var/www/drupal-7-pareview/pareview_temp/jquery_carousel.module
--------------------------------------------------------------------------------
FOUND 3 ERRORS AND 1 WARNING AFFECTING 4 LINES
--------------------------------------------------------------------------------
95 | WARNING | Line exceeds 80 characters; contains 90 characters
106 | ERROR | Doc comment short description must end with a full stop
114 | ERROR | Doc comment short description must end with a full stop
121 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
--------------------------------------------------------------------------------

FILE: /var/www/drupal-7-pareview/pareview_temp/jquery_carousel.info
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
6 | ERROR | Declared file was not found
--------------------------------------------------------------------------------

Manual Review

jquery_carousel_requirements() : Correct TYPO form from Can be downloaded form http://github.com/richardscarrott/jquery-ui-carousel'.

jquery_carousel_field_formatter_settings_form(): Remove commented code // $jquery_carousel_form['#validate'][] = array('jquery_carousel_config_form_validate');

_jquery_carousel_include_css_js(): Using #attached is preferred over drupal_add_library(), drupal_add_js(), and drupal_add_js().

But that are not critical application blockers. Blocking issues from #27 and #32 have been addressed. Been sitting at RTBC for a while now, so...

er.pushpinderrana’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for your contribution, piyuesh23!

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:

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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.