Description
The FlexSlider Field Collection module provides a field formatter to render field collection entities in a FlexSlider.

Requirements
https://drupal.org/project/flexslider
https://drupal.org/project/field_collection
https://drupal.org/project/ctools

Project page
https://drupal.org/sandbox/genjohnson/2063343

Git repository
git clone http://git.drupal.org/sandbox/genjohnson/2063343.git flexslider_field_collection

Projects I've reviewed
Bongo https://drupal.org/node/2075353#comment-7934501
Previous/Next Navigation https://drupal.org/node/2111671#comment-7969411
Apache Solr Multisite Filter https://drupal.org/node/2112607#comment-7969507

Comments

garnett2125’s picture

Hi,

Manual review :

  • Maybe you should add ctools in the dependencies of the .info file since you are using ctools_include on line 99 flexslider_field_collection.module
  • The variable $bundle might have not been define on line 189 flexslider_field_collection.module, you should add a !empty($bundle) in your condition

=)

genjohnson’s picture

garnett2125,

Thanks for such a timely review.
I've implemented both of the changes you suggested. :)

klausi’s picture

You need to set the status to "needs review" if you want to get a review, see http://drupal.org/node/532400

genjohnson’s picture

Status: Active » Needs review

klausi, thanks for the reminder!

PA robot’s picture

Status: Needs review » Needs work

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxgenjohnson2063343git

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.

genjohnson’s picture

Status: Needs work » Needs review

I've made a few minor changes based on the automated review tools report.

artem_sylchuk’s picture

Status: Needs review » Needs work
  • Link to git repository are not valid
  • In REQUIREMENTS are no dependencies to modules ctools
  • Master Branch
    It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
  • Comments to fucntion _flexslider_field_collection_get_fields() is not corect
genjohnson’s picture

Status: Needs work » Needs review

@james_kerrigan, thanks for the feedback. I've made the changes you recommended.

kscheirer’s picture

Status: Needs review » Needs work
Project page
Please take a moment to make your project page follow tips for a great project page.

Otherwise this looks ready to go. There's a bunch of flexslider modules out now, can you mention some of the differences on your project page and fill it out in general?

----
Top Shelf Modules - Crafted, Curated, Contributed.

genjohnson’s picture

Status: Needs work » Needs review

@kscheirer, thanks for the review. I've updated the project page.

kscheirer’s picture

Status: Needs review » Reviewed & tested by the community

Looks nice, thanks!

----
Top Shelf Modules - Crafted, Curated, Contributed.

kscheirer’s picture

Issue summary: View changes

Adding link to CTools requirement and correcting link to git repository.

genjohnson’s picture

Issue summary: View changes

Adding modules I've reviewed

genjohnson’s picture

Issue tags: +PAreview: review bonus

Adding "PAReview: review bonus" tag.

klausi’s picture

Status: Reviewed & tested by the community » Fixed

manual review:

  1. flexslider_field_collection_field_formatter_settings_form(): no need to run $optionsets through check_plain() since #options in select form elements will be sanitized by the form API already, and double escaping is bad. Make sure to read https://drupal.org/node/28984 again.
  2. _flexslider_field_collection_get_fields(): that switch() statement does not make sense, only one case?

But that are not blockers, so ...

Thanks for your contribution, genjohnson!

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.

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

Anonymous’s picture

Issue summary: View changes

Reviewed Apache Solr Multisite Filter