[D7] OMS Flexslider

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

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.

The OMS Flexslider seems to be a useful module, but there are still things to be fixed:
Automatic review:The number of errors are discovered by vental.org (http://ventral.org/pareview/httpgitdrupalorgsandboxjasonrichardsmith2009...).
Manual review:
In oms_flexslider_block_configure() function you include the includes/oms_flexslider.block_form.inc file. In this file you initialize the $form array and then simply return it from the oms_flexslider_block_configure() function.
I think this would be better to wrap the code in oms_flexslider.block_form.inc into a function and call it, e.g:
$form = oms_flexslider_block_configure_form();

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://drupal.org/node/2009900

Project 2: https://drupal.org/node/2009846

Project 3: https://drupal.org/node/1388672

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

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

Hey Jason,

Great module but it does have an issue.

Although the text of OMS slider content is saved as plain text, it is not rendered as plain text, meaning that I can put something like malicious javascript inside the text box and have it run. Take a look at the check_plain() function to eliminate the rendering of html elements wherever field_oms_slider_text is put on the page.

Project page

Please take a moment to make your project page follow tips for a great project page.

PAReview: 3rd party code

jquery.flexslider.js and flexslider.css appear 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.

Possible Duplication

It seems that Flexslider provides very similar functionality. Could you describe how your module differs?

We prefer collaboration over competition, therefore we want to prevent having duplicating modules on drupal.org. If the differences between these modules are not too fundamental for patching the 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).

If that fails for whatever reason please get back to us and set this back to "needs review".

You have a few issues listed here: http://pareview.sh/pareview/httpgitdrupalorgsandboxjasonrichardsmith2009..., ignore the ones generated by flexslider code of course.
Remove .gitignore file from the repo.
In your schema 'uid' => array('description' => 'Block ID.', but in oms_flexslider_block_query() it looks like that column is a user id.
You should consider integration with the Bean module, which allows you to create block types (similar to content types but for blocks). That might be easier than handling all the custom portions yourself.

Setting to "needs work" for the first 2 issues, which are blocking.

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

LANGUAGE_NONE is a constant, not a string, so you can use it like $content .= file_create_url($slide->field_oms_slider_image[LANGUAGE_NONE][0]['uri']); (no quotes around it). That should work just the same as 'und'.

The project page still looks the same?

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

Let's get a second opinion on the duplication issue first then.

you said

I do not feel this is duplication, since it does not require views or any other module outside of core to create a flexslider. This is literally a drop in solution.

but it seems http://drupalcode.org/project/flexslider.git/blob/dc3884f:/flexslider.info only depends on libraries (which yours should) and on ctools (exportables?) outside of core, could you elaborate on the differences between your module and flexslider?

on a different note: the bean module might be a bit obscure but its great for multiple fielded blocks so you can easily create a bean type with a flexislider field (like i did with field_slideshow https://drupal.org/sandbox/discipolo/1972366)

edit: i just simplytest.me your project:

1. the block creation is nice! (although there may just as well be an "add new slideblock" link on the structure/blocks page - and the page that lets you define number of blocks might better fit into admin/config - superfish module has a similar page and puts it there )
2. i am getting Notice: Undefined variable: block in oms_flexslider_block_view() (line 196 of /home/s2896e68288a5083/www/sites/default/modules/2009844/oms_flexslider.module). before creating and assigning content to a newly created and positioned block.
3. Notice: Undefined index: data in drupal_process_attached() (line 4494 of /home/s2896e68288a5083/www/includes/common.inc). if library not installed

that said: very intuitive and fast interface to create blocks under admin/structure and assign content to them using a predefined nodetype.
that interface/workflow is distinctly different from the one provided by flexslider on install in that it doesnt require to setup nodes or views.
having one block precreated on install might be sensible cause we can expect users that enable the module will want at least one.
It should still be investigated if flexislider module can use some of this but that may happen once this is approved and being used.

Based on #21 it seems like your module is significantly different. The project page does need to be updated though, this is a blocking issue. And you have a few minor errors that would be nice to take care of, reported at http://pareview.sh/pareview/httpgitdrupalorgsandboxjasonrichardsmith2009....

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

Awesome, thanks!

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

It's been a month with all problems resolved, so...

Thanks for your contribution, jasonrichardsmith@gmail.com!

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

Here are some recommended readings to help with excellent maintainership:

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

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get 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.

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