[D7] Views JQFX Supersized

Welcome,

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Hi thechanceg,

1)Please find the automated review of your module here.
2)I have done manual review of your code and found some issues as listed below.

2.1)In .info file - please remove below code as this will be added by drupal.org

; Information added by drupal.org packaging script on 2011-09-19
version = "7.x-1.x-dev"
core = "7.x"
project = "views_jqfx_supersized"
datestamp = "1316435393"

2.2)In .module file you have used incorrect commenting for eg comments like hook_theme is above 'views_jqfx_supersized_init' function which should be above 'views_jqfx_supersized_theme' function.
2.3)In views_jqfx_supersized.views_jqfx.inc file - Use standard commenting function which are available in api.drupal.org.

Hii,
Please look into the error report generated with the automated code sniffer:
http://ventral.org/pareview/httpgitdrupalorgsandboxthechanceg1358600git

These are mostly indentation and formatting issues but theres a lot of them. Please fix them.
here is a gist of the issues pointed out by code sniffer:-

1. Do not use tabs to indent lines. instead use spaces.
2. In css files, style definitions must end with a semicolon.
3. Multiple CSS properties should be listed in alphabetical order.
4. Lots of spacing issues found in js files.

Thanks.

supersized folder: 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.

Exceptions can only be made if the library had to be modified to work with Drupal and the modifications were not accepted by the original author. Please read http://drupal.org/node/422996 again.

@thechanceg

I did manual review. In code everything seems to be fine except one thing.
In views-jqfx-supersized.tpl.php

<div id="controls-wrapper" class="load-item">
		<div id="controls">

			<a id="play-button"><img id="pauseplay" src="/sites/all/libraries/supersized/slideshow/img/pause.png"/></a>

			<!--Slide counter-->
			<div id="slidecounter">
				<span class="slidenumber"></span> / <span class="totalslides"></span>
			</div>

			<!--Slide captions displayed here-->
			<div id="slidecaption"></div>

			<!--Thumb Tray button-->
			<a id="tray-button"><img id="tray-arrow" src="/sites/all/libraries/supersized/slideshow/img/button-tray-up.png"/></a>

			<!--Navigation-->
			<ul id="slide-list"></ul>

		</div>
	</div>

Please don't use hard coded paths for images. Instead of this use base_path() path to get the base path of drupal installation. The above code will not work if some one have drupal installed in a sub directory .

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

If you reopen this please keep in mind that we are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

You have a couple of minor errors listed here: http://pareview.sh/pareview/httpgitdrupalorgsandboxthechanceg1358600git.

Looks good though, nice plugin! You could improve the Libraries API support (I believe a hook_libraries_info()) and use their api to detect the path to the jquery files instead of hard-coding them on the template. No major issues found though.

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

Sorry for the delay, but you have not listed any reviews of other project applications in your issue summary as strongly recommended in the application documentation.

manual review:

1. views_jqfx_supersized_init(): Why do you need that library on ever single page request? Shouldn't you only add it if it is needed? And if it really must be on every page you should use hook_page_build() instead.
2. views_jqfx_supersized_views_jqfx_options_form_validate(): I think you can use #element_validate => array('element_validate_integer_positive') on the form element instead, then you don't have to validate that yourself.

but that are not blockers, so ...

Thanks for your contribution, thechanceg!

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:

• 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 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.