Rijkshuisstijl Slider

Hi,

Please add the git direct link so reviewers can easily clone your project :-)

It would be nice to add a more explicit description of your module, what it does, what can be cofigure on it etc so potential users can have information before installing it.

A screenshot of the module in action would be great also.

You should delete the git master branch.

Why have you duplicate and separate the t('strings') ? Isn't that confusing if at some moment you want to change a text string on the module you will have to do it twice.

At class.php, line 33. The a tags can be done with the l() function.
l($slide_index + 1, urlencode( "#slide-" . ( $slide_index + 1 ) ) );

Thanks for the screenshot and the explanation, but is not to me that you have to give it but to the potential users of the module.
Al that info should go into the sandbox page.
Please read again Tips for a great project page

The master branch is still there.
Please read http://drupal.org/empty-git-master

Line 64 of the RijkshuisstijlSliderBean.class.php: You are trusting the URL value using $_GET and inserting it directly into a l() function without checking it before. That is a security issue.
Please check that all data from non trustable sources that are rendered are properly checked.
See this to get more info: http://drupal.org/node/28984

Line 64 of the RijkshuisstijlSliderBean.class.php:
It would also improve readability to put the condition outside of the l() function...

You should add a README.txt file on the root of the folder.

Please be sure that those issues are solved before changing to needs review.

The PAReview.sh is giving some minor things to solve also:
http://ventral.org/pareview/httpgitdrupalorgsandboxrobinvdvleuten1813090git

From the l() docs http://api.drupal.org/api/drupal/includes!common.inc/function/l/7 :

"string $path: The internal path or external URL being linked to, such as "node/34" or "http://example.com/foo". After the url() function is called to construct the URL from $path and $options, the resulting URL is passed through check_plain() before it is inserted into the HTML anchor tag, to ensure well-formed HTML. See url() for more information and notes."

So the check_plain() is not needed here and results only in double escaping, which is bad. So that is not a security issue here.

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 :-)

I believe this project should be labeled "needs work." Please remove check_plain() from your code and try again.

Also, have you considered adding this functionality into the theme itself?

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 :-)

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" (found some problems with the project) or "reviewed & tested by the community" (found no major flaws).

Added a note about the bean module.